Various fixes.

Makefile

@@ -1,3 +1,17 @@
+#
+# Summary of compiler flags you may want to use:
+#
+# * -DNDEBUG: makes binary smaller and faster by removing sanity checks.
+# * -O3: optimize for maximum speed
+# * -fomit-frame-pointer: makes code smaller and faster by freeing up a reg.
+#
+# Threading:
+# * -DUPB_USE_PTHREADS: configures upb to use pthreads r/w lock.
+# * -DUPB_THREAD_UNSAFE: remove all thread-safety.
+# * -pthread: required on GCC to enable pthreads (but what does it do?)
+#
+# Other:
+# * -DUPB_UNALIGNED_READS_OK: makes code smaller, but not standard compliant
 
 # Function to expand a wildcard pattern recursively.
 rwildcard=$(strip $(foreach d,$(wildcard $1*),$(call rwildcard,$d/,$2)$(filter $(subst *,%,$2),$d)))
@@ -8,13 +22,14 @@ CXX=g++
 CFLAGS=-std=c99
 INCLUDE=-Idescriptor -Isrc -Itests -I.
 CPPFLAGS=-Wall -Wextra -g $(INCLUDE) $(strip $(shell test -f perf-cppflags && cat perf-cppflags))
+LDLIBS=-lpthread
 
 LIBUPB=src/libupb.a
 ALL=deps $(OBJ) $(LIBUPB) tests/test_table tests/tests tools/upbc
 all: $(ALL)
 clean:
-	rm -f $(call rwildcard,,*.o) $(ALL) benchmark/google_messages.proto.pb benchmark/google_messages.pb.* benchmarks/b.* benchmarks/*.pb*
-	rm -f descriptor/descriptor.proto.pb
+	rm -rf $(call rwildcard,,*.o) $(ALL) benchmark/google_messages.proto.pb benchmark/google_messages.pb.* benchmarks/b.* benchmarks/*.pb*
+	rm -rf descriptor/descriptor.proto.pb
 
 # The core library (src/libupb.a)
 OBJ=src/upb_parse.o src/upb_table.o src/upb_msg.o src/upb_enum.o src/upb_context.o \

src/upb_atomic.h

@@ -79,12 +79,12 @@ INLINE void upb_atomic_refcount_init(upb_atomic_refcount_t *a, int val) {
   __sync_synchronize();   /* Ensure the initialized value is visible. */
 }
 
-INLINE void upb_atomic_ref(upb_atomic_refcount_t *a) {
-  return __sync_fetch_and_add(&a->val) == 0;
+INLINE bool upb_atomic_ref(upb_atomic_refcount_t *a) {
+  return __sync_fetch_and_add(&a->val, 1) == 0;
 }
 
 INLINE bool upb_atomic_unref(upb_atomic_refcount_t *a) {
-  return __sync_sub_and_fetch(&a->val) == 0;
+  return __sync_sub_and_fetch(&a->val, 1) == 0;
 }
 
 #elif defined(WIN32)
@@ -119,7 +119,9 @@ INLINE bool upb_atomic_unref(upb_atomic_refcount_t *a) {
 
 /* Already defined. */
 
-#elif defined(_POSIX_THREADS)
+#elif defined(UPB_USE_PTHREADS)
+
+#include <pthread.h>
 
 typedef struct {
   pthread_rwlock_t lock;
@@ -127,7 +129,7 @@ typedef struct {
 
 INLINE void upb_rwlock_init(upb_rwlock_t *l) {
   /* TODO: check return value. */
-  pthread_rwlock_init(&l->lock);
+  pthread_rwlock_init(&l->lock, NULL);
 }
 
 INLINE void upb_rwlock_destroy(upb_rwlock_t *l) {

src/upb_context.c

@@ -12,7 +12,7 @@
 #include "upb_msg.h"
 
 /* Search for a character in a string, in reverse. */
-static int memrchr(char *data, char c, size_t len)
+static int my_memrchr(char *data, char c, size_t len)
 {
   int off = len-1;
   while(off > 0 && data[off] != c) --off;
@@ -69,7 +69,6 @@ static void free_context(struct upb_context *c)
     upb_msg_free((struct upb_msg*)c->fds[i]);
   free_symtab(&c->psymtab);
   free(c->fds);
-  free(c);
 }
 
 void upb_context_unref(struct upb_context *c)
@@ -79,6 +78,7 @@ void upb_context_unref(struct upb_context *c)
     free_context(c);
     upb_rwlock_unlock(&c->lock);
   }
+  free(c);
   upb_rwlock_destroy(&c->lock);
 }
 
@@ -132,7 +132,7 @@ static struct upb_symtab_entry *resolve(struct upb_strtable *t,
       if (e) return e;
       else if(baselen == 0) return NULL;  /* No more scopes to try. */
 
-      baselen = memrchr(base->ptr, UPB_SYMBOL_SEPARATOR, baselen);
+      baselen = my_memrchr(base->ptr, UPB_SYMBOL_SEPARATOR, baselen);
     }
   }
 }

src/upb_msg.c

@@ -339,8 +339,7 @@ struct upb_msgsizes {
 };
 
 /* Declared below -- this and get_valuesize are mutually recursive. */
-static size_t get_msgsize(struct upb_msgsizes *sizes, void *data,
-                          struct upb_msgdef *m);
+static size_t get_msgsize(struct upb_msgsizes *sizes, struct upb_msg *m);
 
 /* Returns a size of a value as it will be serialized.  Does *not* include
  * the size of the tag -- that is already accounted for. */
@@ -351,12 +350,12 @@ static size_t get_valuesize(struct upb_msgsizes *sizes, union upb_value_ptr p,
   switch(f->type) {
     default: assert(false); return 0;  /* Internal corruption. */
     case GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_MESSAGE: {
-      size_t submsg_size = get_msgsize(sizes, p.msg, f->ref.msg);
+      size_t submsg_size = get_msgsize(sizes, *p.msg);
       return upb_get_INT32_size(submsg_size) + submsg_size;
     }
     case GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_GROUP: {
       size_t endgrp_tag_size = upb_get_tag_size(fd->number);
-      return endgrp_tag_size + get_msgsize(sizes, p.msg, f->ref.msg);
+      return endgrp_tag_size + get_msgsize(sizes, *p.msg);
     }
 #define CASE(type, member) \
     case GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_ ## type: \
@@ -382,16 +381,15 @@ static size_t get_valuesize(struct upb_msgsizes *sizes, union upb_value_ptr p,
 /* This is mostly just a pure recursive function to calculate the size of a
  * message.  However it also stores the results of each level of the recursion
  * in sizes, because we need all of this intermediate information later. */
-static size_t get_msgsize(struct upb_msgsizes *sizes, void *data,
-                          struct upb_msgdef *m)
+static size_t get_msgsize(struct upb_msgsizes *sizes, struct upb_msg *m)
 {
   size_t size = 0;
   /* We iterate over fields and arrays in reverse order. */
-  for(int32_t i = m->num_fields - 1; i >= 0; i--) {
-    struct upb_msg_fielddef *f = &m->fields[i];
-    google_protobuf_FieldDescriptorProto *fd = upb_msg_field_descriptor(f, m);
-    if(!upb_msg_isset(data, f)) continue;
-    union upb_value_ptr p = upb_msg_getptr(data, f);
+  for(int32_t i = m->def->num_fields - 1; i >= 0; i--) {
+    struct upb_msg_fielddef *f = &m->def->fields[i];
+    google_protobuf_FieldDescriptorProto *fd = upb_msg_field_descriptor(f, m->def);
+    if(!upb_msg_isset(m, f)) continue;
+    union upb_value_ptr p = upb_msg_getptr(m, f);
     if(upb_isarray(f)) {
       for(int32_t j = (*p.arr)->len - 1; j >= 0; j--) {
         union upb_value_ptr elem = upb_array_getelementptr((*p.arr), j, f->type);
@@ -415,9 +413,9 @@ static size_t get_msgsize(struct upb_msgsizes *sizes, void *data,
   return size;
 }
 
-void upb_msgsizes_read(struct upb_msgsizes *sizes, void *data, struct upb_msgdef *m)
+void upb_msgsizes_read(struct upb_msgsizes *sizes, struct upb_msg *m)
 {
-  get_msgsize(sizes, data, m);
+  get_msgsize(sizes, m);
 }
 
 /* Initialize/free a upb_msg_sizes for the given message. */
@@ -457,11 +455,10 @@ void upb_msg_serialize_free(struct upb_msg_serialize_state *s)
   (void)s;
 }
 
-void upb_msg_serialize_init(struct upb_msg_serialize_state *s, void *data,
-                            struct upb_msgdef *m, struct upb_msgsizes *sizes)
+void upb_msg_serialize_init(struct upb_msg_serialize_state *s, struct upb_msg *m,
+                            struct upb_msgsizes *sizes)
 {
   (void)s;
-  (void)data;
   (void)m;
   (void)sizes;
 }

src/upb_msg.h

@@ -189,9 +189,9 @@ INLINE bool upb_msg_isset(struct upb_msg *msg, struct upb_msg_fielddef *f)
 }
 
 /* Returns true if *all* required fields are set, false otherwise. */
-INLINE bool upb_msg_all_required_fields_set(struct upb_msg *msg, struct upb_msgdef *m)
+INLINE bool upb_msg_all_required_fields_set(struct upb_msg *msg)
 {
-  int num_fields = m->num_required_fields;
+  int num_fields = msg->def->num_required_fields;
   int i = 0;
   while(num_fields > 8) {
     if(msg->data[i++] != 0xFF) return false;
@@ -341,8 +341,7 @@ void upb_msgsizes_free(struct upb_msgsizes *sizes);
 
 /* Given a previously initialized sizes, recurse over the message and store its
  * sizes in 'sizes'. */
-void upb_msgsizes_read(struct upb_msgsizes *sizes, void *msg,
-                       struct upb_msgdef *m);
+void upb_msgsizes_read(struct upb_msgsizes *sizes, struct upb_msg *msg);
 
 /* Returns the total size of the serialized message given in sizes.  Must be
  * preceeded by a call to upb_msgsizes_read. */
@@ -355,8 +354,8 @@ struct upb_msg_serialize_state;
  * "sizes" and the parse being fully completed. */
 void upb_msg_serialize_alloc(struct upb_msg_serialize_state *s);
 void upb_msg_serialize_free(struct upb_msg_serialize_state *s);
-void upb_msg_serialize_init(struct upb_msg_serialize_state *s, void *msg,
-                            struct upb_msgdef *m, struct upb_msgsizes *sizes);
+void upb_msg_serialize_init(struct upb_msg_serialize_state *s,
+                            struct upb_msg *msg, struct upb_msgsizes *sizes);
 
 /* Serializes the next set of bytes into buf (which has size len).  Returns
  * UPB_STATUS_OK if serialization is complete, or UPB_STATUS_NEED_MORE_DATA

tools/upbc.c

@@ -35,7 +35,7 @@ static void to_preproc(struct upb_string *str)
     str->ptr[i] = toupper(str->ptr[i]);
 }
 
-static int memrchr(char *data, char c, size_t len)
+static int my_memrchr(char *data, char c, size_t len)
 {
   int off = len-1;
   while(off > 0 && data[off] != c) --off;
@@ -92,9 +92,9 @@ static void write_h(struct upb_symtab_entry *entries[], int num_entries,
     to_cident(enum_name);
 
     struct upb_string *enum_val_prefix = upb_strdup(&entry->e.key);
-    enum_val_prefix->byte_len = memrchr(enum_val_prefix->ptr,
-                                       UPB_SYMBOL_SEPARATOR,
-                                       enum_val_prefix->byte_len);
+    enum_val_prefix->byte_len = my_memrchr(enum_val_prefix->ptr,
+                                           UPB_SYMBOL_SEPARATOR,
+                                           enum_val_prefix->byte_len);
     enum_val_prefix->byte_len++;
     to_preproc(enum_val_prefix);