From ca4605b3af559d0600e8102dd4255d4a6261720c Mon Sep 17 00:00:00 2001
From: Joshua Haberman <jhaberman@gmail.com>
Date: Wed, 4 Sep 2019 10:54:47 -0700
Subject: Properly cache decoder methods, instead of regenerating every time.

This doesn't fully share bytecode.  If you generate for message A
which has B as a sub-message, then generate B, B's code will be
generated twice.  This could be optimized later if desired.  But
this addresses the leak in PHP.

Also removed some obsolete JIT-only code.
---
 upb/pb/compile_decoder.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

(limited to 'upb/pb/compile_decoder.c')

diff --git a/upb/pb/compile_decoder.c b/upb/pb/compile_decoder.c
index 63d7fe2..c39951c 100644
--- a/upb/pb/compile_decoder.c
+++ b/upb/pb/compile_decoder.c
@@ -822,12 +822,10 @@ static void set_bytecode_handlers(mgroup *g) {
 
 /* TODO(haberman): allow this to be constructed for an arbitrary set of dest
  * handlers and other mgroups (but verify we have a transitive closure). */
-const mgroup *mgroup_new(const upb_handlers *dest, bool allowjit, bool lazy) {
+const mgroup *mgroup_new(const upb_handlers *dest, bool lazy) {
   mgroup *g;
   compiler *c;
 
-  UPB_UNUSED(allowjit);
-
   g = newgroup();
   c = newcompiler(g, lazy);
   find_methods(c, dest);
@@ -872,7 +870,6 @@ upb_pbcodecache *upb_pbcodecache_new(upb_handlercache *dest) {
   if (!c) return NULL;
 
   c->dest = dest;
-  c->allow_jit = true;
   c->lazy = false;
 
   c->arena = upb_arena_new();
@@ -882,13 +879,12 @@ upb_pbcodecache *upb_pbcodecache_new(upb_handlercache *dest) {
 }
 
 void upb_pbcodecache_free(upb_pbcodecache *c) {
-  size_t i;
+  upb_inttable_iter i;
 
-  for (i = 0; i < upb_inttable_count(&c->groups); i++) {
-    upb_value v;
-    bool ok = upb_inttable_lookup(&c->groups, i, &v);
-    UPB_ASSERT(ok);
-    freegroup((void*)upb_value_getconstptr(v));
+  upb_inttable_begin(&i, &c->groups);
+  for(; !upb_inttable_done(&i); upb_inttable_next(&i)) {
+    upb_value val = upb_inttable_iter_value(&i);
+    freegroup((void*)upb_value_getconstptr(val));
   }
 
   upb_inttable_uninit(&c->groups);
@@ -896,15 +892,6 @@ void upb_pbcodecache_free(upb_pbcodecache *c) {
   upb_gfree(c);
 }
 
-bool upb_pbcodecache_allowjit(const upb_pbcodecache *c) {
-  return c->allow_jit;
-}
-
-void upb_pbcodecache_setallowjit(upb_pbcodecache *c, bool allow) {
-  UPB_ASSERT(upb_inttable_count(&c->groups) == 0);
-  c->allow_jit = allow;
-}
-
 void upb_pbdecodermethodopts_setlazy(upb_pbcodecache *c, bool lazy) {
   UPB_ASSERT(upb_inttable_count(&c->groups) == 0);
   c->lazy = lazy;
@@ -917,11 +904,13 @@ const upb_pbdecodermethod *upb_pbcodecache_get(upb_pbcodecache *c,
   const upb_handlers *h;
   const mgroup *g;
 
-  /* Right now we build a new DecoderMethod every time.
-   * TODO(haberman): properly cache methods by their true key. */
   h = upb_handlercache_get(c->dest, md);
-  g = mgroup_new(h, c->allow_jit, c->lazy);
-  upb_inttable_push(&c->groups, upb_value_constptr(g));
+  if (upb_inttable_lookupptr(&c->groups, md, &v)) {
+    g = upb_value_getconstptr(v);
+  } else {
+    g = mgroup_new(h, c->lazy);
+    upb_inttable_push(&c->groups, upb_value_constptr(g));
+  }
 
   ok = upb_inttable_lookupptr(&g->methods, h, &v);
   UPB_ASSERT(ok);
-- 
cgit v1.2.3