Add startseq/endseq handlers.

Startseq/endseq handlers are called at the beginning
and end of a sequence of repeated values.  Protobuf
does not really have direct support for this (repeated
primitive fields do not delimit "begin" and "end" of
the sequence) but we can infer them from the bytestream.

The benefit of supporting them explicitly is that they
get their own stack frame and closure, so we can avoid
having to find the array's address over and over and
deciding if we need to initialize it.

This will also pave the way for better support of JSON,
which does have explicit "startseq/endseq" markers: [].

7 files changed, 214 insertions, 123 deletions

diff --git a/src/upb_decoder.c b/src/upb_decoder.c
index 5bb148e..68fb7a5 100644
--- a/src/upb_decoder.c
+++ b/src/upb_decoder.c
@@ -12,6 +12,10 @@
 #include "upb_decoder.h"
 #include "upb_varint.h"
 
+// Used for frames that have no specific end offset: groups, repeated primitive
+// fields inside groups, and the top-level message.
+#define UPB_NONDELIMITED UINT32_MAX
+
 #ifdef UPB_USE_JIT_X64
 #define Dst_DECL upb_decoder *d
 #define Dst_REF (d->dynasm)
@@ -20,11 +24,6 @@
 #include "upb_decoder_x86.h"
 #endif
 
-// A group continues until an END_GROUP tag is seen.
-#define UPB_GROUPEND UINT32_MAX
-// A non-packed repeated field ends when a diff. field is seen (or submsg end).
-#define UPB_REPEATEDEND (UINT32_MAX-1)
-
 // It's unfortunate that we have to micro-manage the compiler this way,
 // especially since this tuning is necessarily specific to one hardware
 // configuration.  But emperically on a Core i7, performance increases 30-50%
@@ -54,7 +53,7 @@ size_t upb_decoder_offset(upb_decoder *d) {
 
 static void upb_decoder_setmsgend(upb_decoder *d) {
   uint32_t end = d->dispatcher.top->end_offset;
-  d->submsg_end = (end == UINT32_MAX) ? (void*)UINTPTR_MAX : d->buf + end;
+  d->submsg_end = (end == UPB_NONDELIMITED) ? (void*)UINTPTR_MAX : d->buf + end;
 }
 
 // Pulls the next buffer from the bytesrc.  Should be called only when the
@@ -72,7 +71,7 @@ static void upb_pullbuf(upb_decoder *d, bool need) {
   if (last_buf_len != -1) {
     d->buf_stream_offset += last_buf_len;
     for (upb_dispatcher_frame *f = d->dispatcher.stack; f <= d->dispatcher.top; ++f)
-      if (f->end_offset != UINT32_MAX)
+      if (f->end_offset != UPB_NONDELIMITED)
         f->end_offset -= last_buf_len;
   }
   d->buf = upb_string_getrobuf(d->bufstr);
@@ -186,14 +185,6 @@ INLINE upb_string *upb_decode_string(upb_decoder *d) {
   return d->tmp;
 }
 
-INLINE void upb_pop(upb_decoder *d) {
-  //if (d->dispatcher.top->end_offset == UPB_REPEATEDEND)
-  //  upb_dispatch_endseq(&d->dispatcher);
-  d->f = d->dispatcher.top->f;
-  upb_dispatch_endsubmsg(&d->dispatcher);
-  upb_decoder_setmsgend(d);
-}
-
 INLINE void upb_push(upb_decoder *d, upb_fhandlers *f, uint32_t end) {
   upb_dispatch_startsubmsg(&d->dispatcher, f)->end_offset = end;
   upb_decoder_setmsgend(d);
@@ -235,11 +226,12 @@ T(SINT64,   varint,  int64,  upb_zzdec_64)
 T(STRING,   string,  str,    upb_string*)
 
 static void upb_decode_GROUP(upb_decoder *d, upb_fhandlers *f) {
-  upb_push(d, f, UPB_GROUPEND);
+  upb_push(d, f, UPB_NONDELIMITED);
 }
 static void upb_endgroup(upb_decoder *d, upb_fhandlers *f) {
   (void)f;
-  upb_pop(d);
+  upb_dispatch_endsubmsg(&d->dispatcher);
+  upb_decoder_setmsgend(d);
 }
 static void upb_decode_MESSAGE(upb_decoder *d, upb_fhandlers *f) {
   upb_push(d, f, upb_decode_varint32(d, true) + (d->ptr - d->buf));
@@ -257,7 +249,13 @@ static void upb_delimend(upb_decoder *d) {
     upb_seterr(d->status, UPB_ERROR, "Bad submessage end.");
     upb_decoder_exit(d);
   }
-  upb_pop(d);
+
+  if (d->dispatcher.top->is_sequence) {
+    upb_dispatch_endseq(&d->dispatcher);
+  } else {
+    upb_dispatch_endsubmsg(&d->dispatcher);
+  }
+  upb_decoder_setmsgend(d);
 }
 
 static void upb_decoder_enterjit(upb_decoder *d) {
@@ -276,10 +274,25 @@ INLINE upb_fhandlers *upb_decode_tag(upb_decoder *d) {
   while (1) {
     uint32_t tag = upb_decode_varint32(d, false);
     upb_fhandlers *f = upb_dispatcher_lookup(&d->dispatcher, tag);
-    if (f) {
-      d->f = f;
-      return f;
+
+    // There are no explicit "startseq" or "endseq" markers in protobuf
+    // streams, so we have to infer them by noticing when a repeated field
+    // starts or ends.
+    if (d->dispatcher.top->is_sequence && d->dispatcher.top->f != f) {
+      upb_dispatch_endseq(&d->dispatcher);
+      upb_decoder_setmsgend(d);
+    }
+    if (f && f->repeated && d->dispatcher.top->f != f) {
+      // TODO: support packed.
+      assert(upb_issubmsgtype(f->type) || upb_isstringtype(f->type) ||
+             (tag & 0x7) != UPB_WIRE_TYPE_DELIMITED);
+      uint32_t end = d->dispatcher.top->end_offset;
+      upb_dispatch_startseq(&d->dispatcher, f)->end_offset = end;
+      upb_decoder_setmsgend(d);
     }
+    if (f) return f;
+
+    // Unknown field.
     switch (tag & 0x7) {
       case UPB_WIRE_TYPE_VARINT:    upb_decode_varint(d); break;
       case UPB_WIRE_TYPE_32BIT:     upb_decoder_advance(d, 4); break;
@@ -291,20 +304,10 @@ INLINE upb_fhandlers *upb_decode_tag(upb_decoder *d) {
     // TODO: deliver to unknown field callback.
     while (d->ptr >= d->submsg_end) upb_delimend(d);
   }
-
-  // Have to handle both packed and non-packed sequences of primitives.
-  //if (d->dispatcher.top->end_offset == UPB_REPEATEDEND && d->f != f) {
-  //  upb_dispatch_endseq(&d->dispatcher);
-  //} else if (f->is_repeated_primitive) {
-  //  if ((tag & 0x7) == UPB_WIRE_TYPE_DELIMITED) {
-  //    upb_pushseq(d, f, upb_decode_varint32(d, true) + (d->ptr - d->buf));
-  //  } else if (d->f != f) {
-  //    upb_dispatch_startseq(d, f, UPB_REPEATEDEND);
-  //  }
-  //}
 }
 
 void upb_decoder_onexit(upb_decoder *d) {
+  if (d->dispatcher.top->is_sequence) upb_dispatch_endseq(&d->dispatcher);
   if (d->status->code == UPB_EOF && upb_dispatcher_stackempty(&d->dispatcher)) {
     // Normal end-of-file.
     upb_clearerr(d->status);
@@ -336,7 +339,7 @@ static void upb_decoder_skip(void *_d, upb_dispatcher_frame *top,
                              upb_dispatcher_frame *bottom) {
   (void)top;
   upb_decoder *d = _d;
-  if (bottom->end_offset == UINT32_MAX) {
+  if (bottom->end_offset == UPB_NONDELIMITED) {
     // TODO: support skipping groups.
     abort();
   }
@@ -386,7 +389,7 @@ void upb_decoder_init(upb_decoder *d, upb_handlers *handlers) {
 }
 
 void upb_decoder_reset(upb_decoder *d, upb_bytesrc *bytesrc, void *closure) {
-  upb_dispatcher_reset(&d->dispatcher, closure)->end_offset = UINT32_MAX;
+  upb_dispatcher_reset(&d->dispatcher, closure)->end_offset = UPB_NONDELIMITED;
   d->bytesrc = bytesrc;
   d->buf = NULL;
   d->ptr = NULL;
diff --git a/src/upb_decoder_x86.dasc b/src/upb_decoder_x86.dasc
index b7195c2..1780c98 100644
--- a/src/upb_decoder_x86.dasc
+++ b/src/upb_decoder_x86.dasc
@@ -285,6 +285,18 @@ static void upb_decoder_jit_field(upb_decoder *d, uint32_t tag, uint32_t next_ta
   |  cmp  edx, (tag & 0x7)
   |  jne  ->exit_jit     // In the future: could be an unknown field or packed.
   |=>f->jit_pclabel_notypecheck:
+  if (f->repeated) {
+    if (f->startseq != upb_startfield_nop) {
+      |  mov   ARG1_64, CLOSURE
+      |  loadfval f
+      |  callp f->startseq
+    } else {
+      |  mov   rdx, CLOSURE
+    }
+    |  mov   esi, FRAME->end_offset
+    |  pushframe  f, rdx, esi, true
+  }
+
   |1:  // Label for repeating this field.
 
   // Decode the value into arg 3 for the callback.
@@ -382,7 +394,7 @@ static void upb_decoder_jit_field(upb_decoder *d, uint32_t tag, uint32_t next_ta
   // Call callbacks.
   if (upb_issubmsgtype(f->type)) {
     // Call startsubmsg handler (if any).
-    if (f->startsubmsg != upb_startsubmsg_nop) {
+    if (f->startsubmsg != upb_startfield_nop) {
       // upb_sflow_t startsubmsg(void *closure, upb_value fval)
       |  mov   r12d, ARG3_32
       |  callp f->startsubmsg
@@ -396,7 +408,7 @@ static void upb_decoder_jit_field(upb_decoder *d, uint32_t tag, uint32_t next_ta
       |   add   esi, r12d   // = (d->ptr - d->buf) + delim_len
     } else {
       assert(f->type == UPB_TYPE(GROUP));
-      |   mov   esi, -1U
+      |   mov   esi, UPB_NONDELIMITED
     }
     |  pushframe  f, rdx, esi, false
 
@@ -411,7 +423,7 @@ static void upb_decoder_jit_field(upb_decoder *d, uint32_t tag, uint32_t next_ta
     |   popframe
 
     // Call endsubmsg handler (if any).
-    if (f->endsubmsg != upb_endsubmsg_nop) {
+    if (f->endsubmsg != upb_endfield_nop) {
       // upb_flow_t endsubmsg(void *closure, upb_value fval);
       |  mov   ARG1_64, CLOSURE
       |  loadfval  f
@@ -428,6 +440,12 @@ static void upb_decoder_jit_field(upb_decoder *d, uint32_t tag, uint32_t next_ta
   if (f->repeated) {
     |  checktag  tag
     |  je  <1
+    |  popframe
+    if (f->endseq != upb_endfield_nop) {
+      |  mov   ARG1_64, CLOSURE
+      |  loadfval f
+      |  callp f->endseq
+    }
   }
   if (next_tag != 0) {
     |  checktag  next_tag
diff --git a/src/upb_handlers.c b/src/upb_handlers.c
index 0be02aa..7be43ce 100644
--- a/src/upb_handlers.c
+++ b/src/upb_handlers.c
@@ -26,12 +26,12 @@ upb_flow_t upb_value_nop(void *closure, upb_value fval, upb_value val) {
   return UPB_CONTINUE;
 }
 
-upb_sflow_t upb_startsubmsg_nop(void *closure, upb_value fval) {
+upb_sflow_t upb_startfield_nop(void *closure, upb_value fval) {
   (void)fval;
   return UPB_CONTINUE_WITH(closure);
 }
 
-upb_flow_t upb_endsubmsg_nop(void *closure, upb_value fval) {
+upb_flow_t upb_endfield_nop(void *closure, upb_value fval) {
   (void)closure;
   (void)fval;
   return UPB_CONTINUE;
@@ -57,9 +57,11 @@ static upb_fhandlers *_upb_mhandlers_newfield(upb_mhandlers *m, uint32_t n,
   upb_fhandlers *f = upb_inttable_lookup(&m->fieldtab, tag);
   if (f) abort();
   upb_fhandlers new_f = {false, type, repeated,
-      repeated && upb_isprimitivetype(type), n, NULL, UPB_NO_VALUE,
-      &upb_value_nop, &upb_startsubmsg_nop, &upb_endsubmsg_nop, 0, 0, 0, NULL};
-  if (upb_issubmsgtype(type)) new_f.startsubmsg = &upb_startsubmsg_nop;
+      repeated && upb_isprimitivetype(type), n, m, NULL, UPB_NO_VALUE,
+      &upb_value_nop,
+      &upb_startfield_nop, &upb_endfield_nop,
+      &upb_startfield_nop, &upb_endfield_nop,
+      0, 0, 0, NULL};
   upb_inttable_insert(&m->fieldtab, tag, &new_f);
   f = upb_inttable_lookup(&m->fieldtab, tag);
   assert(f);
@@ -170,13 +172,13 @@ upb_mhandlers *upb_handlers_newmsg(upb_handlers *h) {
 
 static upb_fhandlers toplevel_f = {
   false, UPB_TYPE(GROUP), false, false, 0,
-  NULL, // submsg
+  NULL, NULL, // submsg
 #ifdef NDEBUG
   {{0}},
 #else
   {{0}, UPB_VALUETYPE_RAW},
 #endif
-  NULL, NULL, NULL, 0, 0, 0, NULL};
+  NULL, NULL, NULL, NULL, NULL, 0, 0, 0, NULL};
 
 void upb_dispatcher_init(upb_dispatcher *d, upb_handlers *h,
                          upb_skip_handler *skip, upb_exit_handler *exit,
@@ -199,6 +201,7 @@ upb_dispatcher_frame *upb_dispatcher_reset(upb_dispatcher *d, void *closure) {
   d->dispatch_table = &d->msgent->fieldtab;
   d->top = d->stack;
   d->top->closure = closure;
+  d->top->is_sequence = false;
   return d->top;
 }
 
@@ -219,8 +222,59 @@ void upb_dispatch_endmsg(upb_dispatcher *d, upb_status *status) {
   upb_copyerr(status, &d->status);
 }
 
+void indent(upb_dispatcher *d) {
+  for (int i = 0; i < (d->top - d->stack); i++) printf(" ");
+}
+
+void indentm1(upb_dispatcher *d) {
+  for (int i = 0; i < (d->top - d->stack - 1); i++) printf(" ");
+}
+
+upb_dispatcher_frame *upb_dispatch_startseq(upb_dispatcher *d,
+                                            upb_fhandlers *f) {
+  //indent(d);
+  //printf("START SEQ: %d\n", f->number);
+  if((d->top+1) >= d->limit) {
+    upb_seterr(&d->status, UPB_ERROR, "Nesting too deep.");
+    _upb_dispatcher_unwind(d, UPB_BREAK);
+    return d->top;  // Dummy.
+  }
+
+  upb_sflow_t sflow = f->startseq(d->top->closure, f->fval);
+  if (sflow.flow != UPB_CONTINUE) {
+    _upb_dispatcher_unwind(d, sflow.flow);
+    return d->top;  // Dummy.
+  }
+
+  ++d->top;
+  d->top->f = f;
+  d->top->is_sequence = true;
+  d->top->closure = sflow.closure;
+  return d->top;
+}
+
+upb_dispatcher_frame *upb_dispatch_endseq(upb_dispatcher *d) {
+  //indentm1(d);
+  //printf("END SEQ\n");
+  assert(d->top > d->stack);
+  assert(d->top->is_sequence);
+  upb_fhandlers *f = d->top->f;
+  --d->top;
+  upb_flow_t flow = f->endseq(d->top->closure, f->fval);
+  if (flow != UPB_CONTINUE) {
+    printf("YO, UNWINDING!\n");
+    _upb_dispatcher_unwind(d, flow);
+    return d->top;  // Dummy.
+  }
+  d->msgent = d->top->f->submsg ? d->top->f->submsg : d->handlers->msgs[0];
+  d->dispatch_table = &d->msgent->fieldtab;
+  return d->top;
+}
+
 upb_dispatcher_frame *upb_dispatch_startsubmsg(upb_dispatcher *d,
                                                upb_fhandlers *f) {
+  //indent(d);
+  //printf("START SUBMSG: %d\n", f->number);
   if((d->top+1) >= d->limit) {
     upb_seterr(&d->status, UPB_ERROR, "Nesting too deep.");
     _upb_dispatcher_unwind(d, UPB_BREAK);
@@ -244,14 +298,16 @@ upb_dispatcher_frame *upb_dispatch_startsubmsg(upb_dispatcher *d,
 }
 
 upb_dispatcher_frame *upb_dispatch_endsubmsg(upb_dispatcher *d) {
+  //indentm1(d);
+  //printf("END SUBMSG\n");
   assert(d->top > d->stack);
-  void *c = d->top->closure;
+  assert(!d->top->is_sequence);
   upb_fhandlers *f = d->top->f;
+  d->msgent->endmsg(d->top->closure, &d->status);
+  d->msgent = d->top->f->msg;
+  d->dispatch_table = &d->msgent->fieldtab;
   --d->top;
-  d->msgent->endmsg(c, &d->status);
   upb_flow_t flow = f->endsubmsg(d->top->closure, f->fval);
-  d->msgent = d->top->f->submsg ? d->top->f->submsg : d->handlers->msgs[0];
-  d->dispatch_table = &d->msgent->fieldtab;
   if (flow != UPB_CONTINUE) _upb_dispatcher_unwind(d, flow);
   return d->top;
 }
diff --git a/src/upb_handlers.h b/src/upb_handlers.h
index 77ea8a8..30908e8 100644
--- a/src/upb_handlers.h
+++ b/src/upb_handlers.h
@@ -68,6 +68,17 @@ extern "C" {
 //     return UPB_CONTINUE;
 //   }
 //
+//   static upb_sflow_t startseqmsg(void *closure, upb_value fval) {
+//     // Called when a sequence (repeated field) begins.  The second element
+//     // of the return value is the closure for the sequence.
+//     return UPB_CONTINUE_WITH(closure);
+//   }
+//
+//   static upb_flow_t endeqvoid *closure, upb_value fval) {
+//     // Called when a sequence ends.
+//     return UPB_CONTINUE;
+//   }
+//
 // All handlers except the endmsg handler return a value from this enum, to
 // control whether parsing will continue or not.
 typedef enum {
@@ -100,16 +111,16 @@ typedef struct _upb_sflow upb_sflow_t;
 typedef upb_flow_t (upb_startmsg_handler)(void *c);
 typedef void (upb_endmsg_handler)(void *c, upb_status *status);
 typedef upb_flow_t (upb_value_handler)(void *c, upb_value fval, upb_value val);
-typedef upb_sflow_t (upb_startsubmsg_handler)(void *closure, upb_value fval);
-typedef upb_flow_t (upb_endsubmsg_handler)(void *closure, upb_value fval);
+typedef upb_sflow_t (upb_startfield_handler)(void *closure, upb_value fval);
+typedef upb_flow_t (upb_endfield_handler)(void *closure, upb_value fval);
 
 // No-op implementations of all of the above handlers.  Use these instead of
 // rolling your own -- the JIT can recognize these and optimize away the call.
 upb_flow_t upb_startmsg_nop(void *closure);
 void upb_endmsg_nop(void *closure, upb_status *status);
 upb_flow_t upb_value_nop(void *closure, upb_value fval, upb_value val);
-upb_sflow_t upb_startsubmsg_nop(void *closure, upb_value fval);
-upb_flow_t upb_endsubmsg_nop(void *closure, upb_value fval);
+upb_sflow_t upb_startfield_nop(void *closure, upb_value fval);
+upb_flow_t upb_endfield_nop(void *closure, upb_value fval);
 
 // Structure definitions.  Do not access any fields directly!  Accessors are
 // provided for the fields that may be get/set.
@@ -136,11 +147,14 @@ typedef struct _upb_fieldent {
   bool repeated;
   bool is_repeated_primitive;
   uint32_t number;
+  upb_mhandlers *msg;
   upb_mhandlers *submsg;  // Must be set iff upb_issubmsgtype(type) == true.
   upb_value fval;
   upb_value_handler *value;
-  upb_startsubmsg_handler *startsubmsg;
-  upb_endsubmsg_handler *endsubmsg;
+  upb_startfield_handler *startsubmsg;
+  upb_endfield_handler *endsubmsg;
+  upb_startfield_handler *startseq;
+  upb_endfield_handler *endseq;
   uint32_t jit_pclabel;
   uint32_t jit_pclabel_notypecheck;
   uint32_t jit_submsg_done_pclabel;
@@ -200,8 +214,10 @@ UPB_MHANDLERS_ACCESSORS(endmsg, upb_endmsg_handler*);
   INLINE type upb_fhandlers_get ## name(upb_fhandlers *f) { return f->name; }
 UPB_FHANDLERS_ACCESSORS(fval, upb_value)
 UPB_FHANDLERS_ACCESSORS(value, upb_value_handler*)
-UPB_FHANDLERS_ACCESSORS(startsubmsg, upb_startsubmsg_handler*)
-UPB_FHANDLERS_ACCESSORS(endsubmsg, upb_endsubmsg_handler*)
+UPB_FHANDLERS_ACCESSORS(startsubmsg, upb_startfield_handler*)
+UPB_FHANDLERS_ACCESSORS(endsubmsg, upb_endfield_handler*)
+UPB_FHANDLERS_ACCESSORS(startseq, upb_startfield_handler*)
+UPB_FHANDLERS_ACCESSORS(endseq, upb_endfield_handler*)
 UPB_FHANDLERS_ACCESSORS(submsg, upb_mhandlers*)
 
 // Convenience function for registering handlers for all messages and
@@ -225,8 +241,10 @@ typedef struct {
   upb_startmsg_handler *startmsg;
   upb_endmsg_handler *endmsg;
   upb_value_handler *value;
-  upb_startsubmsg_handler *startsubmsg;
-  upb_endsubmsg_handler *endsubmsg;
+  upb_startfield_handler *startsubmsg;
+  upb_endfield_handler *endsubmsg;
+  upb_startfield_handler *startseq;
+  upb_endfield_handler *endseq;
 } upb_handlerset;
 
 INLINE void upb_onmreg_hset(void *c, upb_mhandlers *mh, upb_msgdef *m) {
@@ -240,6 +258,8 @@ INLINE void upb_onfreg_hset(void *c, upb_fhandlers *fh, upb_fielddef *f) {
   if (hs->value) upb_fhandlers_setvalue(fh, hs->value);
   if (hs->startsubmsg) upb_fhandlers_setstartsubmsg(fh, hs->startsubmsg);
   if (hs->endsubmsg) upb_fhandlers_setendsubmsg(fh, hs->endsubmsg);
+  if (hs->startseq) upb_fhandlers_setstartseq(fh, hs->startseq);
+  if (hs->endseq) upb_fhandlers_setendseq(fh, hs->endseq);
   upb_value val;
   upb_value_setfielddef(&val, f);
   upb_fhandlers_setfval(fh, val);
@@ -325,6 +345,9 @@ void upb_dispatch_endmsg(upb_dispatcher *d, upb_status *status);
 upb_dispatcher_frame *upb_dispatch_startsubmsg(upb_dispatcher *d,
                                                upb_fhandlers *f);
 upb_dispatcher_frame *upb_dispatch_endsubmsg(upb_dispatcher *d);
+upb_dispatcher_frame *upb_dispatch_startseq(upb_dispatcher *d,
+                                               upb_fhandlers *f);
+upb_dispatcher_frame *upb_dispatch_endseq(upb_dispatcher *d);
 
 #ifdef __cplusplus
 }  /* extern "C" */
diff --git a/src/upb_msg.c b/src/upb_msg.c
index dd00610..9f1f00d 100644
--- a/src/upb_msg.c
+++ b/src/upb_msg.c
@@ -97,7 +97,7 @@ void _upb_array_free(upb_array *arr, upb_fielddef *f) {
   if (upb_elem_ismm(f)) {
     // Need to release refs on sub-objects.
     upb_valuetype_t type = upb_elem_valuetype(f);
-    for (upb_arraylen_t i = 0; i < arr->size; i++) {
+    for (int32_t i = 0; i < arr->size; i++) {
       upb_valueptr p = _upb_array_getptr(arr, f, i);
       upb_elem_unref(upb_value_read(p, type), f);
     }
@@ -117,7 +117,8 @@ void __attribute__((noinline)) upb_array_doresize(
 }
 
 void upb_array_resizefortypesize(upb_array *arr, size_t type_size,
-                                 upb_arraylen_t len) {
+                                 int32_t len) {
+  assert(len >= 0);
   if (arr->size < len) upb_array_doresize(arr, type_size, len);
   arr->len = len;
 }
@@ -312,18 +313,6 @@ static uint64_t upb_msgsink_packfval(uint8_t has_offset, uint8_t has_mask,
   return ret;
 }
 
-upb_valueptr __attribute__((noinline)) upb_array_recycleorresize(
-    upb_msg *m, upb_msgsink_fval fval, size_t type_size) {
-  upb_array **arr = (void*)&m->data[fval.val_offset];
-  if (!(m->data[fval.has_offset] & fval.has_mask)) {
-    upb_array_recycle(arr);
-    m->data[fval.has_offset] |= fval.has_mask;
-  }
-  upb_arraylen_t len = upb_array_len(*arr);
-  upb_array_resizefortypesize(*arr, type_size, len+1);
-  return _upb_array_getptrforsize(*arr, type_size, len);
-}
-
 #define SCALAR_VALUE_CB_PAIR(type, ctype)                                     \
   upb_flow_t upb_msgsink_ ## type ## value(void *_m, upb_value _fval,         \
                                                   upb_value val) {            \
@@ -334,25 +323,13 @@ upb_valueptr __attribute__((noinline)) upb_array_recycleorresize(
     return UPB_CONTINUE;                                                      \
   }                                                                           \
                                                                               \
-  upb_flow_t upb_msgsink_ ## type ## value_r(void *_m, upb_value _fval,       \
-                                                    upb_value val) {          \
-    upb_msg *m = _m;                                                          \
-    upb_msgsink_fval fval = upb_msgsink_unpackfval(_fval);                    \
-    upb_array *arr = *(upb_array**)&m->data[fval.val_offset];                 \
-    if (!(m->data[fval.has_offset] & fval.has_mask)) {                        \
-      if(arr && upb_atomic_only(&arr->refcount)) {                            \
-        m->data[fval.has_offset] |= fval.has_mask;                            \
-        arr->len = 0;                                                         \
-      } else {                                                                \
-        goto slow;                                                            \
-      }                                                                       \
-    } else if (arr->len == arr->size) goto slow;                              \
+  upb_flow_t upb_msgsink_ ## type ## value_r(void *_a, upb_value _fval,       \
+                                             upb_value val) {                 \
+    (void)_fval;                                                              \
+    upb_array *arr = _a;                                                      \
+    upb_array_resizefortypesize(arr, sizeof(ctype), arr->len+1);              \
     upb_valueptr p = _upb_array_getptrforsize(arr, sizeof(ctype),             \
-                                              arr->len++);                    \
-    *(ctype*)p._void = upb_value_get ## type(val);                            \
-    return UPB_CONTINUE;                                                      \
-  slow:                                                                       \
-    p = upb_array_recycleorresize(m, fval, sizeof(ctype));                    \
+                                              arr->len-1);                    \
     *(ctype*)p._void = upb_value_get ## type(val);                            \
     return UPB_CONTINUE;                                                      \
   }                                                                           \
@@ -365,6 +342,17 @@ SCALAR_VALUE_CB_PAIR(uint32, uint32_t)
 SCALAR_VALUE_CB_PAIR(uint64, uint64_t)
 SCALAR_VALUE_CB_PAIR(bool, bool)
 
+upb_sflow_t upb_msgsink_startseq(void *_m, upb_value _fval) {
+  upb_msg *m = _m;
+  upb_msgsink_fval fval = upb_msgsink_unpackfval(_fval);
+  upb_array **arr = (upb_array**)&m->data[fval.val_offset];
+  if (!(m->data[fval.has_offset] & fval.has_mask)) {
+    upb_array_recycle(arr);
+    m->data[fval.has_offset] |= fval.has_mask;
+  }
+  return UPB_CONTINUE_WITH(*arr);
+}
+
 upb_flow_t upb_msgsink_strvalue(void *_m, upb_value _fval, upb_value val) {
   upb_msg *m = _m;
   upb_msgsink_fval fval = upb_msgsink_unpackfval(_fval);
@@ -388,38 +376,24 @@ upb_flow_t upb_msgsink_strvalue(void *_m, upb_value _fval, upb_value val) {
   return UPB_CONTINUE;
 }
 
-upb_flow_t upb_msgsink_strvalue_r(void *_m, upb_value _fval,
+upb_flow_t upb_msgsink_strvalue_r(void *_a, upb_value _fval,
                                   upb_value val) {
-  upb_msg *m = _m;
-  upb_msgsink_fval fval = upb_msgsink_unpackfval(_fval);
-  upb_array *arr = *(upb_array**)&m->data[fval.val_offset];
-  if (!(m->data[fval.has_offset] & fval.has_mask)) {
-    if(arr && upb_atomic_only(&arr->refcount)) {
-      m->data[fval.has_offset] |= fval.has_mask;
-      arr->len = 0;
-    } else {
-      goto slow;
-    }
-  } else if (arr->len == arr->size) goto slow;
-  ++arr->len;
+  upb_array *arr = _a;
+  (void)_fval;
+  upb_array_resizefortypesize(arr, sizeof(void*), arr->len+1);
   upb_valueptr p = _upb_array_getptrforsize(arr, sizeof(void*),
-                                            upb_array_len(arr));
+                                            upb_array_len(arr)-1);
   upb_string *src = upb_value_getstr(val);
   upb_string_recycle(p.str);
   upb_string_substr(*p.str, src, 0, upb_string_len(src));
   return UPB_CONTINUE;
-slow:
-  p = upb_array_recycleorresize(m, fval, sizeof(void*));
-  src = upb_value_getstr(val);
-  upb_string_recycle(p.str);
-  upb_string_substr(*p.str, src, 0, upb_string_len(src));
-  return UPB_CONTINUE;
 }
 
 
 upb_sflow_t upb_msgsink_startsubmsg(void *_m, upb_value _fval) {
-  upb_msg *m = _m;
+  upb_msg *msg = _m;
   upb_msgsink_fval fval = upb_msgsink_unpackfval(_fval);
+
   upb_msgdef md;
   md.size = fval.msg_size;
   md.set_flags_bytes = fval.set_flags_bytes;
@@ -429,12 +403,20 @@ upb_sflow_t upb_msgsink_startsubmsg(void *_m, upb_value _fval) {
   f.label = UPB_LABEL(OPTIONAL);  // Just not repeated.
   f.type = UPB_TYPE(MESSAGE);
   f.byte_offset = fval.val_offset;
-  return UPB_CONTINUE_WITH(upb_msg_appendmsg(m, &f, &md));
+
+  upb_msg **subm = _upb_msg_getptr(msg, &f).msg;
+  if (!upb_msg_has(msg, &f)) {
+    upb_msg_recycle(subm, &md);
+    upb_msg_sethas(msg, &f);
+  }
+  return UPB_CONTINUE_WITH(*subm);
 }
 
-upb_sflow_t upb_msgsink_startsubmsg_r(void *_m, upb_value _fval) {
-  upb_msg *m = _m;
+upb_sflow_t upb_msgsink_startsubmsg_r(void *_a, upb_value _fval) {
+  upb_array *a = _a;
+  assert(a != NULL);
   upb_msgsink_fval fval = upb_msgsink_unpackfval(_fval);
+
   upb_msgdef md;
   md.size = fval.msg_size;
   md.set_flags_bytes = fval.set_flags_bytes;
@@ -444,7 +426,12 @@ upb_sflow_t upb_msgsink_startsubmsg_r(void *_m, upb_value _fval) {
   f.label = UPB_LABEL(REPEATED);
   f.type = UPB_TYPE(MESSAGE);
   f.byte_offset = fval.val_offset;
-  return UPB_CONTINUE_WITH(upb_msg_appendmsg(m, &f, &md));
+
+  upb_arraylen_t oldlen = upb_array_len(a);
+  upb_array_resize(a, &f, oldlen + 1);
+  upb_valueptr p = _upb_array_getptr(a, &f, oldlen);
+  upb_msg_recycle(p.msg, &md);
+  return UPB_CONTINUE_WITH(*p.msg);
 }
 
 INLINE void upb_msg_onfreg(void *c, upb_fhandlers *fh, upb_fielddef *f) {
@@ -459,10 +446,11 @@ INLINE void upb_msg_onfreg(void *c, upb_fhandlers *fh, upb_fielddef *f) {
   upb_value_setuint64(&fh->fval,
       upb_msgsink_packfval(f->set_bit_offset, f->set_bit_mask,
                            f->byte_offset, msg_size, set_flags_bytes));
+  if (fh->repeated) upb_fhandlers_setstartseq(fh, upb_msgsink_startseq);
 #define CASE(upb_type, type) \
 case UPB_TYPE(upb_type): \
-    fh->value = upb_isarray(f) ? \
-        upb_msgsink_ ## type ## value_r : upb_msgsink_ ## type ## value; \
+    upb_fhandlers_setvalue(fh, upb_isarray(f) ? \
+        upb_msgsink_ ## type ## value_r : upb_msgsink_ ## type ## value); \
     break;
   switch (f->type) {
     CASE(DOUBLE,   double)
@@ -484,8 +472,8 @@ case UPB_TYPE(upb_type): \
 #undef CASE
     case UPB_TYPE(MESSAGE):
     case UPB_TYPE(GROUP):
-      fh->startsubmsg =
-          upb_isarray(f) ? upb_msgsink_startsubmsg_r : upb_msgsink_startsubmsg;
+      upb_fhandlers_setstartsubmsg(fh,
+          upb_isarray(f) ? upb_msgsink_startsubmsg_r : upb_msgsink_startsubmsg);
       break;
   }
 }
diff --git a/src/upb_msg.h b/src/upb_msg.h
index a0b2ad2..ac113a8 100644
--- a/src/upb_msg.h
+++ b/src/upb_msg.h
@@ -144,14 +144,15 @@ typedef uint32_t upb_arraylen_t;
 struct _upb_array {
   upb_atomic_refcount_t refcount;
   // "len" and "size" are measured in elements, not bytes.
-  upb_arraylen_t len;
-  upb_arraylen_t size;
+  int32_t len;
+  int32_t size;
   char *ptr;
 };
 
 void _upb_array_free(upb_array *a, upb_fielddef *f);
 INLINE upb_valueptr _upb_array_getptrforsize(upb_array *a, size_t type_size,
-                                             uint32_t elem) {
+                                             int32_t elem) {
+  assert(elem >= 0);
   upb_valueptr p;
   p._void = &a->ptr[elem * type_size];
   return p;
diff --git a/src/upb_textprinter.c b/src/upb_textprinter.c
index bbf7919..ec76d56 100644
--- a/src/upb_textprinter.c
+++ b/src/upb_textprinter.c
@@ -192,7 +192,9 @@ upb_mhandlers *upb_textprinter_reghandlers(upb_handlers *h, upb_msgdef *m) {
     NULL,  // endmsg
     upb_textprinter_value,
     upb_textprinter_startsubmsg,
-    upb_textprinter_endsubmsg
+    upb_textprinter_endsubmsg,
+    NULL,  // startseq
+    NULL,  // endseq
   };
   return upb_handlers_reghandlerset(h, m, &hset);
 }