From 7935b702c5e66e99b0ad2d0b61bb7a7de44dd2ca Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Fri, 19 Aug 2011 23:42:06 -0700 Subject: More cleanup. --- tests/test_table.cc | 19 +++++++------------ upb/pb/decoder.c | 53 +++++++++++++++-------------------------------------- upb/upb.c | 1 - 3 files changed, 22 insertions(+), 51 deletions(-) diff --git a/tests/test_table.cc b/tests/test_table.cc index 8d98f2c..68e8325 100644 --- a/tests/test_table.cc +++ b/tests/test_table.cc @@ -54,7 +54,6 @@ void test_strtable(const vector& keys, uint32_t num_to_insert) for(uint32_t i = 0; i < keys.size(); i++) { const string& key = keys[i]; strtable_entry *e = (strtable_entry*)upb_strtable_lookup(&table, key.c_str()); - printf("Looking up %s...\n", key.c_str()); if(m.find(key) != m.end()) { /* Assume map implementation is correct. */ assert(e); assert(e->value == key[0]); @@ -79,7 +78,7 @@ void test_strtable(const vector& keys, uint32_t num_to_insert) } /* num_entries must be a power of 2. */ -void test_inttable(int32_t *keys, uint16_t num_entries) +void test_inttable(int32_t *keys, uint16_t num_entries, const char *desc) { /* Initialize structures. */ upb_inttable table; @@ -133,6 +132,8 @@ void test_inttable(int32_t *keys, uint16_t num_entries) return; } + printf("%s\n", desc); + /* Test performance. We only test lookups for keys that are known to exist. */ uint16_t rand_order[num_entries]; for(uint16_t i = 0; i < num_entries; i++) { @@ -257,21 +258,16 @@ int main(int argc, char *argv[]) test_strtable(keys, 18); - printf("Benchmarking hash lookups in an integer-keyed hash table.\n"); - printf("\n"); int32_t *keys1 = get_contiguous_keys(8); - printf("Table size: 8, keys: 1-8 ====\n"); - test_inttable(keys1, 8); + test_inttable(keys1, 8, "Table size: 8, keys: 1-8 ===="); delete[] keys1; int32_t *keys2 = get_contiguous_keys(64); - printf("Table size: 64, keys: 1-64 ====\n"); - test_inttable(keys2, 64); + test_inttable(keys2, 64, "Table size: 64, keys: 1-64 ====\n"); delete[] keys2; int32_t *keys3 = get_contiguous_keys(512); - printf("Table size: 512, keys: 1-512 ====\n"); - test_inttable(keys3, 512); + test_inttable(keys3, 512, "Table size: 512, keys: 1-512 ====\n"); delete[] keys3; int32_t *keys4 = new int32_t[64]; @@ -281,7 +277,6 @@ int main(int argc, char *argv[]) else keys4[i] = 10101+i; } - printf("Table size: 64, keys: 1-32 and 10133-10164 ====\n"); - test_inttable(keys4, 64); + test_inttable(keys4, 64, "Table size: 64, keys: 1-32 and 10133-10164 ====\n"); delete[] keys4; } diff --git a/upb/pb/decoder.c b/upb/pb/decoder.c index 2083849..a8ddf17 100644 --- a/upb/pb/decoder.c +++ b/upb/pb/decoder.c @@ -30,8 +30,8 @@ #define NOINLINE static __attribute__((noinline)) static void upb_decoder_exit(upb_decoder *d) { - // If/when we support resumable decoding, we would want to back our progress - // up to completed_ptr and possibly get a previous buffer. + // Resumable decoder would back out to completed_ptr (and possibly get a + // previous buffer). siglongjmp(d->exitjmp, 1); } static void upb_decoder_exit2(void *_d) { @@ -46,9 +46,9 @@ static void upb_decoder_abort(upb_decoder *d, const char *msg) { /* Buffering ******************************************************************/ // We operate on one buffer at a time, which may be a subset of the bytesrc -// region we have ref'd. When data for the buffer is gone we pull the next -// one. When we've committed our progress we release our ref on any previous -// buffers' regions. +// region we have ref'd. When data for the buffer is completely gone we pull +// the next one. When we've committed our progress we release our ref on any +// previous buffers' regions. static size_t upb_decoder_bufleft(upb_decoder *d) { return d->end - d->ptr; } static void upb_decoder_advance(upb_decoder *d, size_t len) { @@ -66,16 +66,10 @@ static void upb_decoder_setmsgend(upb_decoder *d) { upb_dispatcher_frame *f = d->dispatcher.top; size_t delimlen = f->end_ofs - d->bufstart_ofs; size_t buflen = d->end - d->buf; - if (f->end_ofs != UPB_NONDELIMITED && delimlen <= buflen) { - // Delimited message ends in this buffer. - d->delim_end = d->buf + delimlen; - } else { - d->delim_end = NULL; - } + d->delim_end = (f->end_ofs != UPB_NONDELIMITED && delimlen <= buflen) ? + d->buf + delimlen : NULL; // NULL if not in this buf. } -// Pulls the next buffer from the bytesrc. Should be called only when the -// current buffer is completely empty. static bool upb_trypullbuf(upb_decoder *d) { assert(upb_decoder_bufleft(d) == 0); if (d->bufend_ofs == d->refend_ofs) { @@ -84,7 +78,7 @@ static bool upb_trypullbuf(upb_decoder *d) { d->ptr = NULL; d->end = NULL; if (upb_iseof(d->status)) return false; - upb_decoder_exit(d); + upb_decoder_exit(d); // Non-EOF error. } } d->bufstart_ofs = d->bufend_ofs; @@ -295,9 +289,7 @@ static void upb_decode_MESSAGE(upb_decoder *d, upb_fhandlers *f) { static void upb_decoder_checkdelim(upb_decoder *d) { while (d->delim_end != NULL && d->ptr >= d->delim_end) { - if (d->ptr > d->delim_end) - upb_decoder_abort(d, "Bad submessage end"); - + if (d->ptr > d->delim_end) upb_decoder_abort(d, "Bad submessage end"); if (d->dispatcher.top->is_sequence) { upb_dispatch_endseq(&d->dispatcher); } else { @@ -420,27 +412,12 @@ void upb_decoder_initforhandlers(upb_decoder *d, upb_handlers *handlers) { for(upb_inttable_iter i = upb_inttable_begin(&m->fieldtab); !upb_inttable_done(i); i = upb_inttable_next(&m->fieldtab, i)) { upb_fhandlers *f = upb_inttable_iter_value(i); - switch (f->type) { - case UPB_TYPE(INT32): f->decode = &upb_decode_INT32; break; - case UPB_TYPE(INT64): f->decode = &upb_decode_INT64; break; - case UPB_TYPE(UINT32): f->decode = &upb_decode_UINT32; break; - case UPB_TYPE(UINT64): f->decode = &upb_decode_UINT64; break; - case UPB_TYPE(FIXED32): f->decode = &upb_decode_FIXED32; break; - case UPB_TYPE(FIXED64): f->decode = &upb_decode_FIXED64; break; - case UPB_TYPE(SFIXED32): f->decode = &upb_decode_SFIXED32; break; - case UPB_TYPE(SFIXED64): f->decode = &upb_decode_SFIXED64; break; - case UPB_TYPE(BOOL): f->decode = &upb_decode_BOOL; break; - case UPB_TYPE(ENUM): f->decode = &upb_decode_ENUM; break; - case UPB_TYPE(DOUBLE): f->decode = &upb_decode_DOUBLE; break; - case UPB_TYPE(FLOAT): f->decode = &upb_decode_FLOAT; break; - case UPB_TYPE(SINT32): f->decode = &upb_decode_SINT32; break; - case UPB_TYPE(SINT64): f->decode = &upb_decode_SINT64; break; - case UPB_TYPE(STRING): f->decode = &upb_decode_STRING; break; - case UPB_TYPE(BYTES): f->decode = &upb_decode_STRING; break; - case UPB_TYPE(GROUP): f->decode = &upb_decode_GROUP; break; - case UPB_TYPE(MESSAGE): f->decode = &upb_decode_MESSAGE; break; - case UPB_TYPE_ENDGROUP: f->decode = &upb_endgroup; break; - } +#define F(type) &upb_decode_ ## type + static void *fptrs[] = {&upb_endgroup, F(DOUBLE), F(FLOAT), F(INT64), + F(UINT64), F(INT32), F(FIXED64), F(FIXED32), F(BOOL), F(STRING), + F(GROUP), F(MESSAGE), F(STRING), F(UINT32), F(ENUM), F(SFIXED32), + F(SFIXED64), F(SINT32), F(SINT64)}; + f->decode = fptrs[f->type]; } } } diff --git a/upb/upb.c b/upb/upb.c index 0ff082f..006734e 100644 --- a/upb/upb.c +++ b/upb/upb.c @@ -38,7 +38,6 @@ const upb_type_info upb_types[] = { TYPE_INFO(UPB_WIRE_TYPE_64BIT, int64_t, INT64) // SFIXED64 TYPE_INFO(UPB_WIRE_TYPE_VARINT, int32_t, INT32) // SINT32 TYPE_INFO(UPB_WIRE_TYPE_VARINT, int64_t, INT64) // SINT64 - TYPE_INFO(UPB_WIRE_TYPE_END_GROUP, void*, INT64) // SINT64 }; #ifdef NDEBUG -- cgit v1.2.3