Merge pull request #373 from haberman/table-cleanup

Now that handlers are gone, cleaned up table to use arenas exclusively.

benchmarks/compare.py

@@ -58,7 +58,7 @@ def Benchmark(outbase, bench_cpu=True, runs=12, fasttable=False):
 
 
 baseline = "master"
-bench_cpu = False
+bench_cpu = True
 fasttable = False
 
 if len(sys.argv) > 1:

tests/test_table.cc

@@ -14,6 +14,7 @@
 #include <vector>
 
 #include "tests/upb_test.h"
+#include "upb/upb.hpp"
 #include "upb/table.int.h"
 
 #include "upb/port_def.inc"
@@ -25,12 +26,10 @@ namespace upb {
 
 template <class T> upb_value MakeUpbValue(T val);
 template <class T> T GetUpbValue(upb_value val);
-template <class T> upb_ctype_t GetUpbValueType();
 
 #define FUNCS(name, type_t, enumval) \
   template<> upb_value MakeUpbValue<type_t>(type_t val) { return upb_value_ ## name(val); } \
   template<> type_t GetUpbValue<type_t>(upb_value val) { return upb_value_get ## name(val); } \
-  template<> upb_ctype_t GetUpbValueType<type_t>() { return enumval; }
 
 FUNCS(int32,    int32_t,      UPB_CTYPE_INT32)
 FUNCS(int64,    int64_t,      UPB_CTYPE_INT64)
@@ -46,13 +45,12 @@ FUNCS(fptr,     upb_func*,    UPB_CTYPE_FPTR)
 
 class IntTable {
  public:
-  IntTable(upb_ctype_t value_type) { upb_inttable_init(&table_, value_type); }
-  ~IntTable() { upb_inttable_uninit(&table_); }
+  IntTable() { upb_inttable_init(&table_, arena_.ptr()); }
 
   size_t count() { return upb_inttable_count(&table_); }
 
   bool Insert(uintptr_t key, upb_value val) {
-    return upb_inttable_insert(&table_, key, val);
+    return upb_inttable_insert(&table_, key, val, arena_.ptr());
   }
 
   bool Replace(uintptr_t key, upb_value val) {
@@ -73,11 +71,11 @@ class IntTable {
 
   std::pair<bool, upb_value> Lookup32(uint32_t key) const {
     std::pair<bool, upb_value> ret;
-    ret.first = upb_inttable_lookup32(&table_, key, &ret.second);
+    ret.first = upb_inttable_lookup(&table_, key, &ret.second);
     return ret;
   }
 
-  void Compact() { upb_inttable_compact(&table_); }
+  void Compact() { upb_inttable_compact(&table_, arena_.ptr()); }
 
   class iterator : public std::iterator<std::forward_iterator_tag,
                                         std::pair<uintptr_t, upb_value> > {
@@ -115,24 +113,25 @@ class IntTable {
     upb_inttable_iter iter_;
   };
 
+  upb::Arena arena_;
   upb_inttable table_;
 };
 
 class StrTable {
  public:
-  StrTable(upb_ctype_t value_type) { upb_strtable_init(&table_, value_type); }
-  ~StrTable() { upb_strtable_uninit(&table_); }
+  StrTable() { upb_strtable_init(&table_, 4, arena_.ptr()); }
 
   size_t count() { return upb_strtable_count(&table_); }
 
   bool Insert(const std::string& key, upb_value val) {
-    return upb_strtable_insert2(&table_, key.c_str(), key.size(), val);
+    return upb_strtable_insert(&table_, key.c_str(), key.size(), val,
+                               arena_.ptr());
   }
 
   std::pair<bool, upb_value> Remove(const std::string& key) {
     std::pair<bool, upb_value> ret;
     ret.first =
-        upb_strtable_remove2(&table_, key.c_str(), key.size(), &ret.second);
+        upb_strtable_remove(&table_, key.c_str(), key.size(), &ret.second);
     return ret;
   }
 
@@ -144,7 +143,7 @@ class StrTable {
   }
 
   void Resize(size_t size_lg2) {
-    upb_strtable_resize(&table_, size_lg2, &upb_alloc_global);
+    upb_strtable_resize(&table_, size_lg2, arena_.ptr());
   }
 
   class iterator : public std::iterator<std::forward_iterator_tag,
@@ -184,13 +183,12 @@ class StrTable {
     upb_strtable_iter iter_;
   };
 
+  upb::Arena arena_;
   upb_strtable table_;
 };
 
 template <class T> class TypedStrTable {
  public:
-  TypedStrTable() : table_(GetUpbValueType<T>()) {}
-
   size_t count() { return table_.count(); }
 
   bool Insert(const std::string &key, T val) {
@@ -260,8 +258,6 @@ template <class T> class TypedStrTable {
 
 template <class T> class TypedIntTable {
  public:
-  TypedIntTable() : table_(GetUpbValueType<T>()) {}
-
   size_t count() { return table_.count(); }
 
   bool Insert(uintptr_t key, T val) {
@@ -507,7 +503,7 @@ void test_inttable(int32_t *keys, uint16_t num_entries, const char *desc) {
     MAYBE_BREAK;
     int32_t key = keys[i & mask];
     upb_value v;
-    bool ok = upb_inttable_lookup32(&table.table_.table_, key, &v);
+    bool ok = upb_inttable_lookup(&table.table_.table_, key, &v);
     x += (uintptr_t)ok;
   }
   double total = get_usertime() - before;
@@ -521,7 +517,7 @@ void test_inttable(int32_t *keys, uint16_t num_entries, const char *desc) {
     MAYBE_BREAK;
     int32_t key = keys[rand_order[i & mask]];
     upb_value v;
-    bool ok = upb_inttable_lookup32(&table.table_.table_, key, &v);
+    bool ok = upb_inttable_lookup(&table.table_.table_, key, &v);
     x += (uintptr_t)ok;
   }
   total = get_usertime() - before;
@@ -599,12 +595,13 @@ int32_t *get_contiguous_keys(int32_t num) {
 }
 
 void test_delete() {
+  upb::Arena arena;
   upb_inttable t;
-  upb_inttable_init(&t, UPB_CTYPE_BOOL);
-  upb_inttable_insert(&t, 0, upb_value_bool(true));
-  upb_inttable_insert(&t, 2, upb_value_bool(true));
-  upb_inttable_insert(&t, 4, upb_value_bool(true));
-  upb_inttable_compact(&t);
+  upb_inttable_init(&t, arena.ptr());
+  upb_inttable_insert(&t, 0, upb_value_bool(true), arena.ptr());
+  upb_inttable_insert(&t, 2, upb_value_bool(true), arena.ptr());
+  upb_inttable_insert(&t, 4, upb_value_bool(true), arena.ptr());
+  upb_inttable_compact(&t, arena.ptr());
   upb_inttable_remove(&t, 0, NULL);
   upb_inttable_remove(&t, 2, NULL);
   upb_inttable_remove(&t, 4, NULL);
@@ -614,17 +611,15 @@ void test_delete() {
        upb_inttable_next(&iter)) {
     ASSERT(false);
   }
-
-  upb_inttable_uninit(&t);
 }
 
 void test_init() {
   for (int i = 0; i < 2048; i++) {
     /* Tests that the size calculations in init() (lg2 size for target load)
      * work for all expected sizes. */
+    upb::Arena arena;
     upb_strtable t;
-    upb_strtable_init2(&t, UPB_CTYPE_BOOL, i, &upb_alloc_global);
-    upb_strtable_uninit(&t);
+    upb_strtable_init(&t, i, arena.ptr());
   }
 }
 

upb/def.c

@@ -272,8 +272,7 @@ bool upb_enumdef_ntoi(const upb_enumdef *def, const char *name,
 
 const char *upb_enumdef_iton(const upb_enumdef *def, int32_t num) {
   upb_value v;
-  return upb_inttable_lookup32(&def->iton, num, &v) ?
-      upb_value_getcstr(v) : NULL;
+  return upb_inttable_lookup(&def->iton, num, &v) ? upb_value_getcstr(v) : NULL;
 }
 
 const char *upb_enum_iter_name(upb_enum_iter *iter) {
@@ -526,8 +525,8 @@ upb_syntax_t upb_msgdef_syntax(const upb_msgdef *m) {
 
 const upb_fielddef *upb_msgdef_itof(const upb_msgdef *m, uint32_t i) {
   upb_value val;
-  return upb_inttable_lookup32(&m->itof, i, &val) ?
-      upb_value_getconstptr(val) : NULL;
+  return upb_inttable_lookup(&m->itof, i, &val) ? upb_value_getconstptr(val)
+                                                : NULL;
 }
 
 const upb_fielddef *upb_msgdef_ntof(const upb_msgdef *m, const char *name,
@@ -735,8 +734,8 @@ const upb_fielddef *upb_oneofdef_ntof(const upb_oneofdef *o,
 
 const upb_fielddef *upb_oneofdef_itof(const upb_oneofdef *o, uint32_t num) {
   upb_value val;
-  return upb_inttable_lookup32(&o->itof, num, &val) ?
-      upb_value_getptr(val) : NULL;
+  return upb_inttable_lookup(&o->itof, num, &val) ? upb_value_getptr(val)
+                                                  : NULL;
 }
 
 void upb_oneof_begin(upb_oneof_iter *iter, const upb_oneofdef *o) {
@@ -826,8 +825,8 @@ upb_symtab *upb_symtab_new(void) {
   s->bytes_loaded = 0;
   alloc = upb_arena_alloc(s->arena);
 
-  if (!upb_strtable_init2(&s->syms, UPB_CTYPE_CONSTPTR, 32, alloc) ||
-      !upb_strtable_init2(&s->files, UPB_CTYPE_CONSTPTR, 4, alloc)) {
+  if (!upb_strtable_init(&s->syms, 32, s->arena) ||
+      !upb_strtable_init(&s->files, 4, s->arena)) {
     upb_arena_free(s->arena);
     upb_gfree(s);
     s = NULL;
@@ -883,8 +882,7 @@ int upb_symtab_filecount(const upb_symtab *s) {
 typedef struct {
   upb_symtab *symtab;
   upb_filedef *file;              /* File we are building. */
-  upb_arena *file_arena;          /* Allocate defs here. */
-  upb_alloc *alloc;               /* Alloc of file_arena, for tables. */
+  upb_arena *arena;               /* Allocate defs here. */
   const upb_msglayout **layouts;  /* NULL if we should build layouts. */
   upb_status *status;             /* Record errors here. */
   jmp_buf err;                    /* longjmp() on error. */
@@ -906,7 +904,7 @@ static void symtab_oomerr(symtab_addctx *ctx) {
 }
 
 void *symtab_alloc(symtab_addctx *ctx, size_t bytes) {
-  void *ret = upb_arena_malloc(ctx->file_arena, bytes);
+  void *ret = upb_arena_malloc(ctx->arena, bytes);
   if (!ret) symtab_oomerr(ctx);
   return ret;
 }
@@ -1179,7 +1177,7 @@ static void make_layout(symtab_addctx *ctx, const upb_msgdef *m) {
 }
 
 static char *strviewdup(symtab_addctx *ctx, upb_strview view) {
-  return upb_strdup2(view.data, view.size, ctx->alloc);
+  return upb_strdup2(view.data, view.size, ctx->arena);
 }
 
 static bool streql2(const char *a, size_t n, const char *b) {
@@ -1290,9 +1288,9 @@ static void symtab_add(symtab_addctx *ctx, const char *name, upb_value v) {
   if (upb_strtable_lookup(&ctx->symtab->syms, name, NULL)) {
     symtab_errf(ctx, "duplicate symbol '%s'", name);
   }
-  upb_alloc *alloc = upb_arena_alloc(ctx->symtab->arena);
   size_t len = strlen(name);
-  CHK_OOM(upb_strtable_insert3(&ctx->symtab->syms, name, len, v, alloc));
+  CHK_OOM(upb_strtable_insert(&ctx->symtab->syms, name, len, v,
+                              ctx->symtab->arena));
 }
 
 /* Given a symbol and the base symbol inside which it is defined, find the
@@ -1344,10 +1342,10 @@ static void create_oneofdef(
 
   v = pack_def(o, UPB_DEFTYPE_ONEOF);
   symtab_add(ctx, o->full_name, v);
-  CHK_OOM(upb_strtable_insert3(&m->ntof, name.data, name.size, v, ctx->alloc));
+  CHK_OOM(upb_strtable_insert(&m->ntof, name.data, name.size, v, ctx->arena));
 
-  CHK_OOM(upb_inttable_init2(&o->itof, UPB_CTYPE_CONSTPTR, ctx->alloc));
-  CHK_OOM(upb_strtable_init2(&o->ntof, UPB_CTYPE_CONSTPTR, 4, ctx->alloc));
+  CHK_OOM(upb_inttable_init(&o->itof, ctx->arena));
+  CHK_OOM(upb_strtable_init(&o->ntof, 4, ctx->arena));
 }
 
 static str_t *newstr(symtab_addctx *ctx, const char *data, size_t len) {
@@ -1504,7 +1502,6 @@ static void set_default_default(symtab_addctx *ctx, upb_fielddef *f) {
 static void create_fielddef(
     symtab_addctx *ctx, const char *prefix, upb_msgdef *m,
     const google_protobuf_FieldDescriptorProto *field_proto) {
-  upb_alloc *alloc = ctx->alloc;
   upb_fielddef *f;
   const google_protobuf_FieldOptions *options;
   upb_strview name;
@@ -1562,12 +1559,12 @@ static void create_fielddef(
     v = upb_value_constptr(f);
     json_size = strlen(json_name);
 
-    CHK_OOM(
-        upb_strtable_insert3(&m->ntof, name.data, name.size, field_v, alloc));
-    CHK_OOM(upb_inttable_insert2(&m->itof, field_number, v, alloc));
+    CHK_OOM(upb_strtable_insert(&m->ntof, name.data, name.size, field_v,
+                                ctx->arena));
+    CHK_OOM(upb_inttable_insert(&m->itof, field_number, v, ctx->arena));
 
     if (strcmp(shortname, json_name) != 0) {
-      upb_strtable_insert3(&m->ntof, json_name, json_size, json_v, alloc);
+      upb_strtable_insert(&m->ntof, json_name, json_size, json_v, ctx->arena);
     }
 
     if (ctx->layouts) {
@@ -1630,15 +1627,16 @@ static void create_fielddef(
       symtab_errf(ctx, "oneof_index out of range (%s)", f->full_name);
     }
 
-    oneof = (upb_oneofdef*)&m->oneofs[oneof_index];
+    oneof = (upb_oneofdef *)&m->oneofs[oneof_index];
     f->oneof = oneof;
 
     oneof->field_count++;
     if (f->proto3_optional_) {
       oneof->synthetic = true;
     }
-    CHK_OOM(upb_inttable_insert2(&oneof->itof, f->number_, v, alloc));
-    CHK_OOM(upb_strtable_insert3(&oneof->ntof, name.data, name.size, v, alloc));
+    CHK_OOM(upb_inttable_insert(&oneof->itof, f->number_, v, ctx->arena));
+    CHK_OOM(
+        upb_strtable_insert(&oneof->ntof, name.data, name.size, v, ctx->arena));
   } else {
     f->oneof = NULL;
     if (f->proto3_optional_) {
@@ -1681,8 +1679,8 @@ static void create_enumdef(
   symtab_add(ctx, e->full_name, pack_def(e, UPB_DEFTYPE_ENUM));
 
   values = google_protobuf_EnumDescriptorProto_value(enum_proto, &n);
-  CHK_OOM(upb_strtable_init2(&e->ntoi, UPB_CTYPE_INT32, n, ctx->alloc));
-  CHK_OOM(upb_inttable_init2(&e->iton, UPB_CTYPE_CSTR, ctx->alloc));
+  CHK_OOM(upb_strtable_init(&e->ntoi, n, ctx->arena));
+  CHK_OOM(upb_inttable_init(&e->iton, ctx->arena));
 
   e->file = ctx->file;
   e->defaultval = 0;
@@ -1709,16 +1707,15 @@ static void create_enumdef(
     }
 
     CHK_OOM(name2)
-    CHK_OOM(
-        upb_strtable_insert3(&e->ntoi, name2, strlen(name2), v, ctx->alloc));
+    CHK_OOM(upb_strtable_insert(&e->ntoi, name2, strlen(name2), v, ctx->arena));
 
     if (!upb_inttable_lookup(&e->iton, num, NULL)) {
       upb_value v = upb_value_cstr(name2);
-      CHK_OOM(upb_inttable_insert2(&e->iton, num, v, ctx->alloc));
+      CHK_OOM(upb_inttable_insert(&e->iton, num, v, ctx->arena));
     }
   }
 
-  upb_inttable_compact2(&e->iton, ctx->alloc);
+  upb_inttable_compact(&e->iton, ctx->arena);
 }
 
 static void create_msgdef(symtab_addctx *ctx, const char *prefix,
@@ -1742,9 +1739,8 @@ static void create_msgdef(symtab_addctx *ctx, const char *prefix,
   oneofs = google_protobuf_DescriptorProto_oneof_decl(msg_proto, &n_oneof);
   fields = google_protobuf_DescriptorProto_field(msg_proto, &n_field);
 
-  CHK_OOM(upb_inttable_init2(&m->itof, UPB_CTYPE_CONSTPTR, ctx->alloc));
-  CHK_OOM(upb_strtable_init2(&m->ntof, UPB_CTYPE_CONSTPTR, n_oneof + n_field,
-                             ctx->alloc));
+  CHK_OOM(upb_inttable_init(&m->itof, ctx->arena));
+  CHK_OOM(upb_strtable_init(&m->ntof, n_oneof + n_field, ctx->arena));
 
   m->file = ctx->file;
   m->map_entry = false;
@@ -1778,7 +1774,7 @@ static void create_msgdef(symtab_addctx *ctx, const char *prefix,
 
   finalize_oneofs(ctx, m);
   assign_msg_wellknowntype(m);
-  upb_inttable_compact2(&m->itof, ctx->alloc);
+  upb_inttable_compact(&m->itof, ctx->arena);
 
   /* This message is built.  Now build nested messages and enums. */
 
@@ -2007,19 +2003,18 @@ static void build_filedef(
 }
 
 static void remove_filedef(upb_symtab *s, upb_filedef *file) {
-  upb_alloc *alloc = upb_arena_alloc(s->arena);
   int i;
   for (i = 0; i < file->msg_count; i++) {
     const char *name = file->msgs[i].full_name;
-    upb_strtable_remove3(&s->syms, name, strlen(name), NULL, alloc);
+    upb_strtable_remove(&s->syms, name, strlen(name), NULL);
   }
   for (i = 0; i < file->enum_count; i++) {
     const char *name = file->enums[i].full_name;
-    upb_strtable_remove3(&s->syms, name, strlen(name), NULL, alloc);
+    upb_strtable_remove(&s->syms, name, strlen(name), NULL);
   }
   for (i = 0; i < file->ext_count; i++) {
     const char *name = file->exts[i].full_name;
-    upb_strtable_remove3(&s->syms, name, strlen(name), NULL, alloc);
+    upb_strtable_remove(&s->syms, name, strlen(name), NULL);
   }
 }
 
@@ -2037,8 +2032,7 @@ static const upb_filedef *_upb_symtab_addfile(
 
   ctx.file = file;
   ctx.symtab = s;
-  ctx.file_arena = file_arena;
-  ctx.alloc = upb_arena_alloc(file_arena);
+  ctx.arena = file_arena;
   ctx.layouts = layouts;
   ctx.status = status;
 
@@ -2053,8 +2047,8 @@ static const upb_filedef *_upb_symtab_addfile(
     file = NULL;
   } else {
     build_filedef(&ctx, file, file_proto);
-    upb_strtable_insert3(&s->files, file->name, strlen(file->name),
-                         upb_value_constptr(file), ctx.alloc);
+    upb_strtable_insert(&s->files, file->name, strlen(file->name),
+                        upb_value_constptr(file), ctx.arena);
     UPB_ASSERT(upb_ok(status));
     upb_arena_fuse(s->arena, file_arena);
   }

upb/msg.c

@@ -133,7 +133,7 @@ upb_map *_upb_map_new(upb_arena *a, size_t key_size, size_t value_size) {
     return NULL;
   }
 
-  upb_strtable_init2(&map->table, UPB_CTYPE_INT32, 4, upb_arena_alloc(a));
+  upb_strtable_init(&map->table, 4, a);
   map->key_size = key_size;
   map->val_size = value_size;
 

upb/msg.h

@@ -460,20 +460,19 @@ UPB_INLINE void* _upb_map_next(const upb_map *map, size_t *iter) {
 }
 
 UPB_INLINE bool _upb_map_set(upb_map *map, const void *key, size_t key_size,
-                             void *val, size_t val_size, upb_arena *arena) {
+                             void *val, size_t val_size, upb_arena *a) {
   upb_strview strkey = _upb_map_tokey(key, key_size);
   upb_value tabval = {0};
-  if (!_upb_map_tovalue(val, val_size, &tabval, arena)) return false;
-  upb_alloc *a = upb_arena_alloc(arena);
+  if (!_upb_map_tovalue(val, val_size, &tabval, a)) return false;
 
   /* TODO(haberman): add overwrite operation to minimize number of lookups. */
-  upb_strtable_remove3(&map->table, strkey.data, strkey.size, NULL, a);
-  return upb_strtable_insert3(&map->table, strkey.data, strkey.size, tabval, a);
+  upb_strtable_remove(&map->table, strkey.data, strkey.size, NULL);
+  return upb_strtable_insert(&map->table, strkey.data, strkey.size, tabval, a);
 }
 
 UPB_INLINE bool _upb_map_delete(upb_map *map, const void *key, size_t key_size) {
   upb_strview k = _upb_map_tokey(key, key_size);
-  return upb_strtable_remove3(&map->table, k.data, k.size, NULL, NULL);
+  return upb_strtable_remove(&map->table, k.data, k.size, NULL);
 }
 
 UPB_INLINE void _upb_map_clear(upb_map *map) {
@@ -538,8 +537,7 @@ UPB_INLINE void _upb_msg_map_key(const void* msg, void* key, size_t size) {
 
 UPB_INLINE void _upb_msg_map_value(const void* msg, void* val, size_t size) {
   const upb_tabent *ent = (const upb_tabent*)msg;
-  upb_value v;
-  _upb_value_setval(&v, ent->val.val);
+  upb_value v = {ent->val.val};
   _upb_map_fromvalue(v, val, size);
 }
 

upb/table.c

@@ -27,6 +27,12 @@ static const double MIN_DENSITY = 0.1;
 
 bool is_pow2(uint64_t v) { return v == 0 || (v & (v - 1)) == 0; }
 
+static upb_value _upb_value_val(uint64_t val) {
+  upb_value ret;
+  _upb_value_setval(&ret, val);
+  return ret;
+}
+
 int log2ceil(uint64_t v) {
   int ret = 0;
   bool pow2 = is_pow2(v);
@@ -35,11 +41,11 @@ int log2ceil(uint64_t v) {
   return UPB_MIN(UPB_MAXARRSIZE, ret);
 }
 
-char *upb_strdup(const char *s, upb_alloc *a) {
+char *upb_strdup(const char *s, upb_arena *a) {
   return upb_strdup2(s, strlen(s), a);
 }
 
-char *upb_strdup2(const char *s, size_t len, upb_alloc *a) {
+char *upb_strdup2(const char *s, size_t len, upb_arena *a) {
   size_t n;
   char *p;
 
@@ -48,7 +54,7 @@ char *upb_strdup2(const char *s, size_t len, upb_alloc *a) {
   /* Always null-terminate, even if binary data; but don't rely on the input to
    * have a null-terminating byte since it may be a raw binary buffer. */
   n = len + 1;
-  p = upb_malloc(a, n);
+  p = upb_arena_malloc(a, n);
   if (p) {
     memcpy(p, s, len);
     p[len] = 0;
@@ -83,16 +89,24 @@ typedef bool eqlfunc_t(upb_tabkey k1, lookupkey_t k2);
 
 /* Base table (shared code) ***************************************************/
 
-/* For when we need to cast away const. */
-static upb_tabent *mutable_entries(upb_table *t) {
-  return (upb_tabent*)t->entries;
+static uint32_t upb_inthash(uintptr_t key) {
+  return (uint32_t)key;
+}
+
+static const upb_tabent *upb_getentry(const upb_table *t, uint32_t hash) {
+  return t->entries + (hash & t->mask);
+}
+
+static bool upb_arrhas(upb_tabval key) {
+  return key.val != (uint64_t)-1;
 }
 
+
 static bool isfull(upb_table *t) {
   return t->count == t->max_count;
 }
 
-static bool init(upb_table *t, uint8_t size_lg2, upb_alloc *a) {
+static bool init(upb_table *t, uint8_t size_lg2, upb_arena *a) {
   size_t bytes;
 
   t->count = 0;
@@ -101,21 +115,17 @@ static bool init(upb_table *t, uint8_t size_lg2, upb_alloc *a) {
   t->max_count = upb_table_size(t) * MAX_LOAD;
   bytes = upb_table_size(t) * sizeof(upb_tabent);
   if (bytes > 0) {
-    t->entries = upb_malloc(a, bytes);
+    t->entries = upb_arena_malloc(a, bytes);
     if (!t->entries) return false;
-    memset(mutable_entries(t), 0, bytes);
+    memset(t->entries, 0, bytes);
   } else {
     t->entries = NULL;
   }
   return true;
 }
 
-static void uninit(upb_table *t, upb_alloc *a) {
-  upb_free(a, mutable_entries(t));
-}
-
 static upb_tabent *emptyent(upb_table *t, upb_tabent *e) {
-  upb_tabent *begin = mutable_entries(t);
+  upb_tabent *begin = t->entries;
   upb_tabent *end = begin + upb_table_size(t);
   for (e = e + 1; e < end; e++) {
     if (upb_tabent_isempty(e)) return e;
@@ -265,9 +275,9 @@ static size_t begin(const upb_table *t) {
 
 /* A simple "subclass" of upb_table that only adds a hash function for strings. */
 
-static upb_tabkey strcopy(lookupkey_t k2, upb_alloc *a) {
+static upb_tabkey strcopy(lookupkey_t k2, upb_arena *a) {
   uint32_t len = (uint32_t) k2.str.len;
-  char *str = upb_malloc(a, k2.str.len + sizeof(uint32_t) + 1);
+  char *str = upb_arena_malloc(a, k2.str.len + sizeof(uint32_t) + 1);
   if (str == NULL) return 0;
   memcpy(str, &len, sizeof(uint32_t));
   if (k2.str.len) memcpy(str + sizeof(uint32_t), k2.str.str, k2.str.len);
@@ -291,9 +301,7 @@ static bool streql(upb_tabkey k1, lookupkey_t k2) {
   return len == k2.str.len && (len == 0 || memcmp(str, k2.str.str, len) == 0);
 }
 
-bool upb_strtable_init2(upb_strtable *t, upb_ctype_t ctype,
-                        size_t expected_size, upb_alloc *a) {
-  UPB_UNUSED(ctype);  /* TODO(haberman): rm */
+bool upb_strtable_init(upb_strtable *t, size_t expected_size, upb_arena *a) {
   // Multiply by approximate reciprocal of MAX_LOAD (0.85), with pow2 denominator.
   size_t need_entries = (expected_size + 1) * 1204 / 1024;
   UPB_ASSERT(need_entries >= expected_size * 0.85);
@@ -307,14 +315,7 @@ void upb_strtable_clear(upb_strtable *t) {
   memset((char*)t->t.entries, 0, bytes);
 }
 
-void upb_strtable_uninit2(upb_strtable *t, upb_alloc *a) {
-  size_t i;
-  for (i = 0; i < upb_table_size(&t->t); i++)
-    upb_free(a, (void*)t->t.entries[i].key);
-  uninit(&t->t, a);
-}
-
-bool upb_strtable_resize(upb_strtable *t, size_t size_lg2, upb_alloc *a) {
+bool upb_strtable_resize(upb_strtable *t, size_t size_lg2, upb_arena *a) {
   upb_strtable new_table;
   upb_strtable_iter i;
 
@@ -323,17 +324,15 @@ bool upb_strtable_resize(upb_strtable *t, size_t size_lg2, upb_alloc *a) {
   upb_strtable_begin(&i, t);
   for ( ; !upb_strtable_done(&i); upb_strtable_next(&i)) {
     upb_strview key = upb_strtable_iter_key(&i);
-    upb_strtable_insert3(
-        &new_table, key.data, key.size,
-        upb_strtable_iter_value(&i), a);
+    upb_strtable_insert(&new_table, key.data, key.size,
+                        upb_strtable_iter_value(&i), a);
   }
-  upb_strtable_uninit2(t, a);
   *t = new_table;
   return true;
 }
 
-bool upb_strtable_insert3(upb_strtable *t, const char *k, size_t len,
-                          upb_value v, upb_alloc *a) {
+bool upb_strtable_insert(upb_strtable *t, const char *k, size_t len,
+                         upb_value v, upb_arena *a) {
   lookupkey_t key;
   upb_tabkey tabkey;
   uint32_t hash;
@@ -360,19 +359,11 @@ bool upb_strtable_lookup2(const upb_strtable *t, const char *key, size_t len,
   return lookup(&t->t, strkey2(key, len), v, hash, &streql);
 }
 
-bool upb_strtable_remove3(upb_strtable *t, const char *key, size_t len,
-                         upb_value *val, upb_alloc *alloc) {
+bool upb_strtable_remove(upb_strtable *t, const char *key, size_t len,
+                         upb_value *val) {
   uint32_t hash = table_hash(key, len);
   upb_tabkey tabkey;
-  if (rm(&t->t, strkey2(key, len), val, &tabkey, hash, &streql)) {
-    if (alloc) {
-      /* Arena-based allocs don't need to free and won't pass this. */
-      upb_free(alloc, (void*)tabkey);
-    }
-    return true;
-  } else {
-    return false;
-  }
+  return rm(&t->t, strkey2(key, len), val, &tabkey, hash, &streql);
 }
 
 /* Iteration */
@@ -470,7 +461,7 @@ static void check(upb_inttable *t) {
 }
 
 bool upb_inttable_sizedinit(upb_inttable *t, size_t asize, int hsize_lg2,
-                            upb_alloc *a) {
+                            upb_arena *a) {
   size_t array_bytes;
 
   if (!init(&t->t, hsize_lg2, a)) return false;
@@ -479,9 +470,8 @@ bool upb_inttable_sizedinit(upb_inttable *t, size_t asize, int hsize_lg2,
   t->array_size = UPB_MAX(1, asize);
   t->array_count = 0;
   array_bytes = t->array_size * sizeof(upb_value);
-  t->array = upb_malloc(a, array_bytes);
+  t->array = upb_arena_malloc(a, array_bytes);
   if (!t->array) {
-    uninit(&t->t, a);
     return false;
   }
   memset(mutable_array(t), 0xff, array_bytes);
@@ -489,18 +479,12 @@ bool upb_inttable_sizedinit(upb_inttable *t, size_t asize, int hsize_lg2,
   return true;
 }
 
-bool upb_inttable_init2(upb_inttable *t, upb_ctype_t ctype, upb_alloc *a) {
-  UPB_UNUSED(ctype);  /* TODO(haberman): rm */
+bool upb_inttable_init(upb_inttable *t, upb_arena *a) {
   return upb_inttable_sizedinit(t, 0, 4, a);
 }
 
-void upb_inttable_uninit2(upb_inttable *t, upb_alloc *a) {
-  uninit(&t->t, a);
-  upb_free(a, mutable_array(t));
-}
-
-bool upb_inttable_insert2(upb_inttable *t, uintptr_t key, upb_value val,
-                          upb_alloc *a) {
+bool upb_inttable_insert(upb_inttable *t, uintptr_t key, upb_value val,
+                         upb_arena *a) {
   upb_tabval tabval;
   tabval.val = val.val;
   UPB_ASSERT(upb_arrhas(tabval));  /* This will reject (uint64_t)-1.  Fix this. */
@@ -531,7 +515,6 @@ bool upb_inttable_insert2(upb_inttable *t, uintptr_t key, upb_value val,
 
       UPB_ASSERT(t->t.count == new_table.count);
 
-      uninit(&t->t, a);
       t->t = new_table;
     }
     insert(&t->t, intkey(key), key, val, upb_inthash(key), &inthash, &inteql);
@@ -575,21 +558,7 @@ bool upb_inttable_remove(upb_inttable *t, uintptr_t key, upb_value *val) {
   return success;
 }
 
-bool upb_inttable_insertptr2(upb_inttable *t, const void *key, upb_value val,
-                             upb_alloc *a) {
-  return upb_inttable_insert2(t, (uintptr_t)key, val, a);
-}
-
-bool upb_inttable_lookupptr(const upb_inttable *t, const void *key,
-                            upb_value *v) {
-  return upb_inttable_lookup(t, (uintptr_t)key, v);
-}
-
-bool upb_inttable_removeptr(upb_inttable *t, const void *key, upb_value *val) {
-  return upb_inttable_remove(t, (uintptr_t)key, val);
-}
-
-void upb_inttable_compact2(upb_inttable *t, upb_alloc *a) {
+void upb_inttable_compact(upb_inttable *t, upb_arena *a) {
   /* A power-of-two histogram of the table keys. */
   size_t counts[UPB_MAXARRSIZE + 1] = {0};
 
@@ -637,12 +606,11 @@ void upb_inttable_compact2(upb_inttable *t, upb_alloc *a) {
     upb_inttable_begin(&i, t);
     for (; !upb_inttable_done(&i); upb_inttable_next(&i)) {
       uintptr_t k = upb_inttable_iter_key(&i);
-      upb_inttable_insert2(&new_t, k, upb_inttable_iter_value(&i), a);
+      upb_inttable_insert(&new_t, k, upb_inttable_iter_value(&i), a);
     }
     UPB_ASSERT(new_t.array_size == arr_size);
     UPB_ASSERT(new_t.t.size_lg2 == hashsize_lg2);
   }
-  upb_inttable_uninit2(t, a);
   *t = new_t;
 }
 

upb/table.int.h

@@ -33,47 +33,20 @@ extern "C" {
 
 /* upb_value ******************************************************************/
 
-/* A tagged union (stored untagged inside the table) so that we can check that
- * clients calling table accessors are correctly typed without having to have
- * an explosion of accessors. */
-typedef enum {
-  UPB_CTYPE_INT32    = 1,
-  UPB_CTYPE_INT64    = 2,
-  UPB_CTYPE_UINT32   = 3,
-  UPB_CTYPE_UINT64   = 4,
-  UPB_CTYPE_BOOL     = 5,
-  UPB_CTYPE_CSTR     = 6,
-  UPB_CTYPE_PTR      = 7,
-  UPB_CTYPE_CONSTPTR = 8,
-  UPB_CTYPE_FPTR     = 9,
-  UPB_CTYPE_FLOAT    = 10,
-  UPB_CTYPE_DOUBLE   = 11
-} upb_ctype_t;
-
 typedef struct {
   uint64_t val;
 } upb_value;
 
 /* Like strdup(), which isn't always available since it's not ANSI C. */
-char *upb_strdup(const char *s, upb_alloc *a);
+char *upb_strdup(const char *s, upb_arena *a);
 /* Variant that works with a length-delimited rather than NULL-delimited string,
  * as supported by strtable. */
-char *upb_strdup2(const char *s, size_t len, upb_alloc *a);
-
-UPB_INLINE char *upb_gstrdup(const char *s) {
-  return upb_strdup(s, &upb_alloc_global);
-}
+char *upb_strdup2(const char *s, size_t len, upb_arena *a);
 
 UPB_INLINE void _upb_value_setval(upb_value *v, uint64_t val) {
   v->val = val;
 }
 
-UPB_INLINE upb_value _upb_value_val(uint64_t val) {
-  upb_value ret;
-  _upb_value_setval(&ret, val);
-  return ret;
-}
-
 /* For each value ctype, define the following set of functions:
  *
  * // Get/set an int32 from a upb_value.
@@ -181,14 +154,7 @@ typedef struct {
   uint32_t mask;         /* Mask to turn hash value -> bucket. */
   uint32_t max_count;    /* Max count before we hit our load limit. */
   uint8_t size_lg2;      /* Size of the hashtable part is 2^size_lg2 entries. */
-
-  /* Hash table entries.
-   * Making this const isn't entirely accurate; what we really want is for it to
-   * have the same const-ness as the table it's inside.  But there's no way to
-   * declare that in C.  So we have to make it const so that we can statically
-   * initialize const hash tables.  Then we cast away const when we have to.
-   */
-  const upb_tabent *entries;
+  upb_tabent *entries;
 } upb_table;
 
 typedef struct {
@@ -202,8 +168,6 @@ typedef struct {
   size_t array_count;       /* Array part number of elements. */
 } upb_inttable;
 
-#define UPB_ARRAY_EMPTYENT -1
-
 UPB_INLINE size_t upb_table_size(const upb_table *t) {
   if (t->size_lg2 == 0)
     return 0;
@@ -216,48 +180,10 @@ UPB_INLINE bool upb_tabent_isempty(const upb_tabent *e) {
   return e->key == 0;
 }
 
-/* Used by some of the unit tests for generic hashing functionality. */
-uint32_t upb_murmur_hash2(const void * key, size_t len, uint32_t seed);
-
-UPB_INLINE uintptr_t upb_intkey(uintptr_t key) {
-  return key;
-}
-
-UPB_INLINE uint32_t upb_inthash(uintptr_t key) {
-  return (uint32_t)key;
-}
-
-static const upb_tabent *upb_getentry(const upb_table *t, uint32_t hash) {
-  return t->entries + (hash & t->mask);
-}
-
-UPB_INLINE bool upb_arrhas(upb_tabval key) {
-  return key.val != (uint64_t)-1;
-}
-
 /* Initialize and uninitialize a table, respectively.  If memory allocation
  * failed, false is returned that the table is uninitialized. */
-bool upb_inttable_init2(upb_inttable *table, upb_ctype_t ctype, upb_alloc *a);
-bool upb_strtable_init2(upb_strtable *table, upb_ctype_t ctype,
-                        size_t expected_size, upb_alloc *a);
-void upb_inttable_uninit2(upb_inttable *table, upb_alloc *a);
-void upb_strtable_uninit2(upb_strtable *table, upb_alloc *a);
-
-UPB_INLINE bool upb_inttable_init(upb_inttable *table, upb_ctype_t ctype) {
-  return upb_inttable_init2(table, ctype, &upb_alloc_global);
-}
-
-UPB_INLINE bool upb_strtable_init(upb_strtable *table, upb_ctype_t ctype) {
-  return upb_strtable_init2(table, ctype, 4, &upb_alloc_global);
-}
-
-UPB_INLINE void upb_inttable_uninit(upb_inttable *table) {
-  upb_inttable_uninit2(table, &upb_alloc_global);
-}
-
-UPB_INLINE void upb_strtable_uninit(upb_strtable *table) {
-  upb_strtable_uninit2(table, &upb_alloc_global);
-}
+bool upb_inttable_init(upb_inttable *table, upb_arena *a);
+bool upb_strtable_init(upb_strtable *table, size_t expected_size, upb_arena *a);
 
 /* Returns the number of values in the table. */
 size_t upb_inttable_count(const upb_inttable *t);
@@ -265,12 +191,6 @@ UPB_INLINE size_t upb_strtable_count(const upb_strtable *t) {
   return t->t.count;
 }
 
-void upb_inttable_packedsize(const upb_inttable *t, size_t *size);
-void upb_strtable_packedsize(const upb_strtable *t, size_t *size);
-upb_inttable *upb_inttable_pack(const upb_inttable *t, void *p, size_t *ofs,
-                                size_t size);
-upb_strtable *upb_strtable_pack(const upb_strtable *t, void *p, size_t *ofs,
-                                size_t size);
 void upb_strtable_clear(upb_strtable *t);
 
 /* Inserts the given key into the hashtable with the given value.  The key must
@@ -280,26 +200,10 @@ void upb_strtable_clear(upb_strtable *t);
  *
  * If a table resize was required but memory allocation failed, false is
  * returned and the table is unchanged. */
-bool upb_inttable_insert2(upb_inttable *t, uintptr_t key, upb_value val,
-                          upb_alloc *a);
-bool upb_strtable_insert3(upb_strtable *t, const char *key, size_t len,
-                          upb_value val, upb_alloc *a);
-
-UPB_INLINE bool upb_inttable_insert(upb_inttable *t, uintptr_t key,
-                                    upb_value val) {
-  return upb_inttable_insert2(t, key, val, &upb_alloc_global);
-}
-
-UPB_INLINE bool upb_strtable_insert2(upb_strtable *t, const char *key,
-                                     size_t len, upb_value val) {
-  return upb_strtable_insert3(t, key, len, val, &upb_alloc_global);
-}
-
-/* For NULL-terminated strings. */
-UPB_INLINE bool upb_strtable_insert(upb_strtable *t, const char *key,
-                                    upb_value val) {
-  return upb_strtable_insert2(t, key, strlen(key), val);
-}
+bool upb_inttable_insert(upb_inttable *t, uintptr_t key, upb_value val,
+                         upb_arena *a);
+bool upb_strtable_insert(upb_strtable *t, const char *key, size_t len,
+                         upb_value val, upb_arena *a);
 
 /* Looks up key in this table, returning "true" if the key was found.
  * If v is non-NULL, copies the value for this key into *v. */
@@ -316,74 +220,21 @@ UPB_INLINE bool upb_strtable_lookup(const upb_strtable *t, const char *key,
 /* Removes an item from the table.  Returns true if the remove was successful,
  * and stores the removed item in *val if non-NULL. */
 bool upb_inttable_remove(upb_inttable *t, uintptr_t key, upb_value *val);
-bool upb_strtable_remove3(upb_strtable *t, const char *key, size_t len,
-                          upb_value *val, upb_alloc *alloc);
-
-UPB_INLINE bool upb_strtable_remove2(upb_strtable *t, const char *key,
-                                     size_t len, upb_value *val) {
-  return upb_strtable_remove3(t, key, len, val, &upb_alloc_global);
-}
-
-/* For NULL-terminated strings. */
-UPB_INLINE bool upb_strtable_remove(upb_strtable *t, const char *key,
-                                    upb_value *v) {
-  return upb_strtable_remove2(t, key, strlen(key), v);
-}
+bool upb_strtable_remove(upb_strtable *t, const char *key, size_t len,
+                          upb_value *val);
 
 /* Updates an existing entry in an inttable.  If the entry does not exist,
  * returns false and does nothing.  Unlike insert/remove, this does not
  * invalidate iterators. */
 bool upb_inttable_replace(upb_inttable *t, uintptr_t key, upb_value val);
 
-/* Convenience routines for inttables with pointer keys. */
-bool upb_inttable_insertptr2(upb_inttable *t, const void *key, upb_value val,
-                             upb_alloc *a);
-bool upb_inttable_removeptr(upb_inttable *t, const void *key, upb_value *val);
-bool upb_inttable_lookupptr(
-    const upb_inttable *t, const void *key, upb_value *val);
-
-UPB_INLINE bool upb_inttable_insertptr(upb_inttable *t, const void *key,
-                                       upb_value val) {
-  return upb_inttable_insertptr2(t, key, val, &upb_alloc_global);
-}
-
 /* Optimizes the table for the current set of entries, for both memory use and
  * lookup time.  Client should call this after all entries have been inserted;
  * inserting more entries is legal, but will likely require a table resize. */
-void upb_inttable_compact2(upb_inttable *t, upb_alloc *a);
-
-UPB_INLINE void upb_inttable_compact(upb_inttable *t) {
-  upb_inttable_compact2(t, &upb_alloc_global);
-}
-
-/* A special-case inlinable version of the lookup routine for 32-bit
- * integers. */
-UPB_INLINE bool upb_inttable_lookup32(const upb_inttable *t, uint32_t key,
-                                      upb_value *v) {
-  *v = upb_value_int32(0);  /* Silence compiler warnings. */
-  if (key < t->array_size) {
-    upb_tabval arrval = t->array[key];
-    if (upb_arrhas(arrval)) {
-      _upb_value_setval(v, arrval.val);
-      return true;
-    } else {
-      return false;
-    }
-  } else {
-    const upb_tabent *e;
-    if (t->t.entries == NULL) return false;
-    for (e = upb_getentry(&t->t, upb_inthash(key)); true; e = e->next) {
-      if ((uint32_t)e->key == key) {
-        _upb_value_setval(v, e->val.val);
-        return true;
-      }
-      if (e->next == NULL) return false;
-    }
-  }
-}
+void upb_inttable_compact(upb_inttable *t, upb_arena *a);
 
 /* Exposed for testing only. */
-bool upb_strtable_resize(upb_strtable *t, size_t size_lg2, upb_alloc *a);
+bool upb_strtable_resize(upb_strtable *t, size_t size_lg2, upb_arena *a);
 
 /* Iterators ******************************************************************/