diff options
| -rw-r--r-- | Makefile | 28 | ||||
| -rw-r--r-- | benchmarks/parsestream.upb.c | 13 | ||||
| -rw-r--r-- | benchmarks/parsetoproto2.upb.cc | 36 | ||||
| -rw-r--r-- | benchmarks/parsetostruct.upb.c | 19 | ||||
| -rw-r--r-- | bindings/lua/upb.c | 24 | ||||
| -rw-r--r-- | bindings/python/upb.c | 10 | ||||
| -rw-r--r-- | tests/test_decoder.c | 2 | ||||
| -rw-r--r-- | tests/test_vs_proto2.cc | 2 | ||||
| -rw-r--r-- | upb/bytestream.c | 4 | ||||
| -rw-r--r-- | upb/bytestream.h | 3 | ||||
| -rw-r--r-- | upb/handlers.c | 1 | ||||
| -rw-r--r-- | upb/pb/decoder.c | 6 | ||||
| -rw-r--r-- | upb/pb/decoder_x64.dasc | 3 | ||||
| -rw-r--r-- | upb/upb.c | 3 | ||||
| -rw-r--r-- | upb/upb.h | 2 |
15 files changed, 92 insertions, 64 deletions
@@ -199,12 +199,10 @@ SIMPLE_TESTS= \ tests/test_varint \ tests/tests \ -INTERACTIVE_TESTS= \ +# Too many tests in this binary to run Valgrind (it takes minutes). +SLOW_TESTS= \ tests/test_decoder \ -# tests/test_stream \ - - SIMPLE_CXX_TESTS= \ tests/test_table \ tests/test_cpp \ @@ -214,9 +212,7 @@ VARIADIC_TESTS= \ tests/t.test_vs_proto2.googlemessage2 \ TESTS=$(SIMPLE_TESTS) $(SIMPLE_CXX_TESTS) $(VARIADIC_TESTS) - - -tests: $(TESTS) $(INTERACTIVE_TESTS) +tests: $(TESTS) $(TESTS): $(LIBUPB) tests/tests: tests/test.proto.pb @@ -228,13 +224,21 @@ VALGRIND=valgrind --leak-check=full --error-exitcode=1 test: tests @echo Running all tests under valgrind. @set -e # Abort on error. - @for test in $(TESTS); do \ + @for test in $(SIMPLE_TESTS) $(SIMPLE_CXX_TESTS); do \ + if [ -x ./$$test ] ; then \ + echo !!! $(VALGRIND) ./$$test tests/test.proto.pb; \ + $(VALGRIND) ./$$test tests/test.proto.pb || exit 1; \ + fi \ + done; + @for test in "$(SLOW_TESTS)"; do \ if [ -x ./$$test ] ; then \ - echo !!! $(VALGRIND) ./$$test; \ - $(VALGRIND) ./$$test || exit 1; \ + echo !!! ./$$test; \ + ./$$test || exit 1; \ fi \ - done; \ - echo "All tests passed!" + done; + @$(VALGRIND) tests/t.test_vs_proto2.googlemessage1 benchmarks/google_messages.proto.pb benchmarks/google_message1.dat + @$(VALGRIND) tests/t.test_vs_proto2.googlemessage2 benchmarks/google_messages.proto.pb benchmarks/google_message2.dat + @echo "All tests passed!" tests/t.test_vs_proto2.googlemessage1 \ tests/t.test_vs_proto2.googlemessage2: \ diff --git a/benchmarks/parsestream.upb.c b/benchmarks/parsestream.upb.c index 4d13e9d..0316a86 100644 --- a/benchmarks/parsestream.upb.c +++ b/benchmarks/parsestream.upb.c @@ -12,6 +12,7 @@ static size_t input_len; static const upb_msgdef *def; static upb_decoder decoder; static upb_stringsrc stringsrc; +static upb_decoderplan *plan; static upb_sflow_t startsubmsg(void *_m, upb_value fval) { (void)_m; @@ -53,11 +54,12 @@ static bool initialize() } upb_handlers *handlers = upb_handlers_new(); - if (!JIT) handlers->should_jit = false; // Cause all messages to be read, but do nothing when they are. upb_handlerset hset = {NULL, NULL, value, startsubmsg, NULL, NULL, NULL}; upb_handlers_reghandlerset(handlers, def, &hset); - upb_decoder_init(&decoder, handlers); + upb_decoder_init(&decoder); + plan = upb_decoderplan_new(handlers, JIT); + upb_decoder_resetplan(&decoder, plan, 0); upb_handlers_unref(handlers); upb_stringsrc_init(&stringsrc); return true; @@ -68,6 +70,7 @@ static void cleanup() free(input_str); upb_def_unref(UPB_UPCAST(def)); upb_decoder_uninit(&decoder); + upb_decoderplan_unref(plan); upb_stringsrc_uninit(&stringsrc); } @@ -76,10 +79,8 @@ static size_t run(int i) (void)i; upb_status status = UPB_STATUS_INIT; upb_stringsrc_reset(&stringsrc, input_str, input_len); - upb_decoder_reset(&decoder, upb_stringsrc_bytesrc(&stringsrc), - 0, UPB_NONDELIMITED, NULL); - upb_decoder_decode(&decoder, &status); - if(!upb_ok(&status)) goto err; + upb_decoder_resetinput(&decoder, upb_stringsrc_allbytes(&stringsrc), NULL); + if (upb_decoder_decode(&decoder) != UPB_OK) goto err; return input_len; err: diff --git a/benchmarks/parsetoproto2.upb.cc b/benchmarks/parsetoproto2.upb.cc index 75cd10c..988faad 100644 --- a/benchmarks/parsetoproto2.upb.cc +++ b/benchmarks/parsetoproto2.upb.cc @@ -30,6 +30,8 @@ MESSAGE_CIDENT msg2; static upb_stringsrc strsrc; static upb_decoder d; static const upb_msgdef *def; +static upb_decoderplan *p; +char *str; #define PROTO2_APPEND(type, ctype) \ upb_flow_t proto2_append_ ## type(void *_r, upb_value fval, upb_value val) { \ @@ -53,9 +55,13 @@ upb_flow_t proto2_setstr(void *m, upb_value fval, upb_value val) { const upb_fielddef *f = upb_value_getfielddef(fval); std::string **str = (std::string**)UPB_INDEX(m, f->offset, 1); if (*str == f->default_ptr) *str = new std::string; - const upb_strref *ref = upb_value_getstrref(val); + const upb_byteregion *reg = upb_value_getbyteregion(val); + size_t len; + (*str)->assign( + upb_byteregion_getptr(reg, upb_byteregion_startofs(reg), &len), + upb_byteregion_len(reg)); // XXX: only supports contiguous strings atm. - (*str)->assign(ref->ptr, ref->len); + assert(len == upb_byteregion_len(reg)); return UPB_CONTINUE; } @@ -64,9 +70,13 @@ upb_flow_t proto2_append_str(void *_r, upb_value fval, upb_value val) { typedef google::protobuf::RepeatedPtrField<std::string> R; (void)fval; R *r = (R*)_r; - const upb_strref *ref = upb_value_getstrref(val); + const upb_byteregion *reg = upb_value_getbyteregion(val); + size_t len; + r->Add()->assign( + upb_byteregion_getptr(reg, upb_byteregion_startofs(reg), &len), + upb_byteregion_len(reg)); // XXX: only supports contiguous strings atm. - r->Add()->assign(ref->ptr, ref->len); + assert(len == upb_byteregion_len(reg)); return UPB_CONTINUE; } @@ -265,7 +275,7 @@ static bool initialize() upb_symtab_unref(s); // Read the message data itself. - char *str = upb_readfile(MESSAGE_FILE, &len); + str = upb_readfile(MESSAGE_FILE, &len); if(str == NULL) { fprintf(stderr, "Error reading " MESSAGE_FILE "\n"); return false; @@ -275,11 +285,11 @@ static bool initialize() msg2.ParseFromArray(str, len); upb_stringsrc_init(&strsrc); - upb_stringsrc_reset(&strsrc, str, len); upb_handlers *h = upb_handlers_new(); upb_accessors_reghandlers(h, def); - if (!JIT) h->should_jit = false; - upb_decoder_init(&d, h); + p = upb_decoderplan_new(h, JIT); + upb_decoder_init(&d); + upb_decoder_resetplan(&d, p, 0); upb_handlers_unref(h); return true; @@ -289,6 +299,8 @@ static void cleanup() { upb_stringsrc_uninit(&strsrc); upb_decoder_uninit(&d); upb_def_unref(UPB_UPCAST(def)); + upb_decoderplan_unref(p); + free(str); } static size_t run(int i) @@ -296,10 +308,10 @@ static size_t run(int i) (void)i; upb_status status = UPB_STATUS_INIT; msg[i % NUM_MESSAGES].Clear(); - upb_decoder_reset(&d, upb_stringsrc_bytesrc(&strsrc), - 0, UPB_NONDELIMITED, &msg[i % NUM_MESSAGES]); - upb_decoder_decode(&d, &status); - if(!upb_ok(&status)) goto err; + upb_stringsrc_reset(&strsrc, str, len); + upb_decoder_resetinput( + &d, upb_stringsrc_allbytes(&strsrc), &msg[i % NUM_MESSAGES]); + if (upb_decoder_decode(&d) != UPB_OK) goto err; return len; err: diff --git a/benchmarks/parsetostruct.upb.c b/benchmarks/parsetostruct.upb.c index 5e7aa35..9487577 100644 --- a/benchmarks/parsetostruct.upb.c +++ b/benchmarks/parsetostruct.upb.c @@ -12,6 +12,8 @@ static size_t len; static void *msg[NUM_MESSAGES]; static upb_stringsrc strsrc; static upb_decoder d; +static upb_decoderplan *p; +char *str; static bool initialize() { @@ -33,7 +35,7 @@ static bool initialize() upb_symtab_unref(s); // Read the message data itself. - char *str = upb_readfile(MESSAGE_FILE, &len); + str = upb_readfile(MESSAGE_FILE, &len); if(str == NULL) { fprintf(stderr, "Error reading " MESSAGE_FILE "\n"); return false; @@ -43,12 +45,12 @@ static bool initialize() msg[i] = upb_stdmsg_new(def); upb_stringsrc_init(&strsrc); - upb_stringsrc_reset(&strsrc, str, len); upb_handlers *h = upb_handlers_new(); upb_accessors_reghandlers(h, def); - if (!JIT) h->should_jit = false; - upb_decoder_init(&d, h); + p = upb_decoderplan_new(h, JIT); + upb_decoder_init(&d); upb_handlers_unref(h); + upb_decoder_resetplan(&d, p, 0); if (!BYREF) { // TODO: use byref/byval accessors. @@ -63,6 +65,8 @@ static void cleanup() upb_def_unref(UPB_UPCAST(def)); upb_stringsrc_uninit(&strsrc); upb_decoder_uninit(&d); + upb_decoderplan_unref(p); + free(str); } static size_t run(int i) @@ -70,10 +74,9 @@ static size_t run(int i) upb_status status = UPB_STATUS_INIT; i %= NUM_MESSAGES; upb_msg_clear(msg[i], def); - upb_decoder_reset(&d, upb_stringsrc_bytesrc(&strsrc), - 0, UPB_NONDELIMITED, msg[i]); - upb_decoder_decode(&d, &status); - if(!upb_ok(&status)) goto err; + upb_stringsrc_reset(&strsrc, str, len); + upb_decoder_resetinput(&d, upb_stringsrc_allbytes(&strsrc), msg[i]); + if (upb_decoder_decode(&d) != UPB_OK) goto err; return len; err: diff --git a/bindings/lua/upb.c b/bindings/lua/upb.c index 4cce4b6..56c5be9 100644 --- a/bindings/lua/upb.c +++ b/bindings/lua/upb.c @@ -37,15 +37,11 @@ static uint32_t lupb_touint32(lua_State *L, int narg, const char *name) { return n; } -static void lupb_pushstring(lua_State *L, const upb_strref *ref) { - if (ref->ptr) { - lua_pushlstring(L, ref->ptr, ref->len); - } else { - // Lua requires a continguous string; must copy+allocate. - char *str = upb_strref_dup(ref); - lua_pushlstring(L, str, ref->len); - free(str); - } +static void lupb_pushstring(lua_State *L, const upb_byteregion *r) { + // TODO: could avoid a copy in the case that the string is contiguous. + char *str = upb_byteregion_strdup(r); + lua_pushlstring(L, str, upb_byteregion_len(r)); + free(str); } static void lupb_pushvalue(lua_State *L, upb_value val, upb_fielddef *f) { @@ -77,7 +73,7 @@ static void lupb_pushvalue(lua_State *L, upb_value val, upb_fielddef *f) { // Returns a scalar value (ie. not a submessage) as a upb_value. static upb_value lupb_getvalue(lua_State *L, int narg, upb_fielddef *f, - upb_strref *ref) { + upb_byteregion *ref) { assert(!upb_issubmsg(f)); upb_value val; if (upb_fielddef_type(f) == UPB_TYPE(BOOL)) { @@ -139,7 +135,7 @@ static upb_value lupb_getvalue(lua_State *L, int narg, upb_fielddef *f, } static void lupb_typecheck(lua_State *L, int narg, upb_fielddef *f) { - upb_strref ref; + upb_byteregion ref; lupb_getvalue(L, narg, f, &ref); } @@ -302,8 +298,8 @@ static void lupb_fielddef_set(lua_State *L, upb_fielddef *f, } else if (streql(field, "default_value")) { if (!upb_fielddef_type(f)) luaL_error(L, "Must set type before setting default_value"); - upb_strref ref; - upb_fielddef_setdefault(f, lupb_getvalue(L, narg, f, &ref)); + upb_byteregion region; + upb_fielddef_setdefault(f, lupb_getvalue(L, narg, f, ®ion)); } else { luaL_error(L, "Cannot set fielddef member '%s'", field); } @@ -782,7 +778,7 @@ static upb_flow_t lupb_msg_string(void *m, upb_value fval, upb_value val, lua_State *L = *(lua_State**)m; int offset = array ? lua_rawlen(L, -1) : f->offset; if (!lua_checkstack(L, 1)) luaL_error(L, "stack full"); - lupb_pushstring(L, upb_value_getstrref(val)); + lupb_pushstring(L, upb_value_getbyteregion(val)); lua_rawseti(L, -2, offset); return UPB_CONTINUE; } diff --git a/bindings/python/upb.c b/bindings/python/upb.c index 497074b..8f36f70 100644 --- a/bindings/python/upb.c +++ b/bindings/python/upb.c @@ -612,8 +612,9 @@ static upb_sflow_t PyUpb_Message_StartRepeatedSubmessage(void *a, upb_value fval static upb_flow_t PyUpb_Message_StringValue(void *m, upb_value fval, upb_value val) { PyObject **str = PyUpb_Accessor_GetPtr(m, fval); if (*str) { Py_DECREF(*str); } - *str = PyString_FromStringAndSize(NULL, upb_value_getstrref(val)->len); - upb_strref_read(upb_value_getstrref(val), PyString_AsString(*str)); + upb_byteregion *r = upb_value_getbyteregion(val); + *str = PyString_FromStringAndSize(NULL, upb_byteregion_len(r)); + upb_byteregion_copyall(r, PyString_AsString(*str)); upb_stdmsg_sethas(m, fval); return UPB_CONTINUE; } @@ -621,8 +622,9 @@ static upb_flow_t PyUpb_Message_StringValue(void *m, upb_value fval, upb_value v static upb_flow_t PyUpb_Message_AppendStringValue(void *a, upb_value fval, upb_value val) { (void)fval; PyObject **elem = upb_stdarray_append(a, sizeof(void*)); - *elem = PyString_FromStringAndSize(NULL, upb_value_getstrref(val)->len); - upb_strref_read(upb_value_getstrref(val), PyString_AsString(*elem)); + upb_byteregion *r = upb_value_getbyteregion(val); + *elem = PyString_FromStringAndSize(NULL, upb_byteregion_len(r)); + upb_byteregion_copyall(r, PyString_AsString(*elem)); return UPB_CONTINUE; } diff --git a/tests/test_decoder.c b/tests/test_decoder.c index 0db3bfa..14d0e2d 100644 --- a/tests/test_decoder.c +++ b/tests/test_decoder.c @@ -165,6 +165,7 @@ upb_sflow_t startsubmsg(void *closure, upb_value fval) { } upb_flow_t endsubmsg(void *closure, upb_value fval) { + (void)fval; buffer_appendf(closure, "} "); return UPB_CONTINUE; } @@ -175,6 +176,7 @@ upb_sflow_t startseq(void *closure, upb_value fval) { } upb_flow_t endseq(void *closure, upb_value fval) { + (void)fval; buffer_appendf(closure, "] "); return UPB_CONTINUE; } diff --git a/tests/test_vs_proto2.cc b/tests/test_vs_proto2.cc index c43649c..53b2498 100644 --- a/tests/test_vs_proto2.cc +++ b/tests/test_vs_proto2.cc @@ -15,7 +15,7 @@ #include <stdlib.h> #include <google/protobuf/descriptor.h> #include <google/protobuf/wire_format_lite.h> -#include "upb/benchmarks/google_messages.pb.h" +#include "benchmarks/google_messages.pb.h" #include "upb/def.h" #include "upb/msg.h" #include "upb/pb/glue.h" diff --git a/upb/bytestream.c b/upb/bytestream.c index 8feb678..4f6e4b1 100644 --- a/upb/bytestream.c +++ b/upb/bytestream.c @@ -91,10 +91,10 @@ static upb_stdio_buf *upb_stdio_findbuf(const upb_stdio *s, uint64_t ofs) { static upb_stdio_buf *upb_stdio_rotatebufs(upb_stdio *s) { upb_stdio_buf **reuse = NULL; // XXX - int num_reused = 0, num_inuse = 0; + uint32_t num_reused = 0, num_inuse = 0; // Could sweep only a subset of bufs if this was a hotspot. - for (int i = 0; i < s->nbuf; i++) { + for (uint32_t i = 0; i < s->nbuf; i++) { upb_stdio_buf *buf = s->bufs[i]; if (buf->refcount > 0) { s->bufs[num_inuse++] = buf; diff --git a/upb/bytestream.h b/upb/bytestream.h index 409ae80..fe049d2 100644 --- a/upb/bytestream.h +++ b/upb/bytestream.h @@ -372,7 +372,8 @@ INLINE int upb_bytesink_putc(upb_bytesink *sink, char ch) { } INLINE int upb_bytesink_putrepeated(upb_bytesink *sink, char ch, int len) { - for (int i = 0; i < len; i++) + int i; + for (i = 0; i < len; i++) if (upb_bytesink_write(sink, &ch, 1) < 0) return -1; return len; diff --git a/upb/handlers.c b/upb/handlers.c index d1b68ad..1ccaf8d 100644 --- a/upb/handlers.c +++ b/upb/handlers.c @@ -187,6 +187,7 @@ upb_dispatcher_frame *upb_dispatcher_reset(upb_dispatcher *d, void *closure, } void upb_dispatcher_uninit(upb_dispatcher *d) { + (void)d; } void upb_dispatch_startmsg(upb_dispatcher *d) { diff --git a/upb/pb/decoder.c b/upb/pb/decoder.c index 1b5fc17..06125dd 100644 --- a/upb/pb/decoder.c +++ b/upb/pb/decoder.c @@ -96,7 +96,12 @@ void upb_decoderplan_unref(upb_decoderplan *p) { } bool upb_decoderplan_hasjitcode(upb_decoderplan *p) { +#ifdef UPB_USE_JIT_X64 return p->jit_code != NULL; +#else + (void)p; + return false; +#endif } @@ -537,6 +542,7 @@ void upb_decoder_resetplan(upb_decoder *d, upb_decoderplan *p, int msg_offset) { void upb_decoder_resetinput(upb_decoder *d, upb_byteregion *input, void *closure) { assert(d->plan); + assert(upb_byteregion_discardofs(input) == upb_byteregion_startofs(input)); upb_dispatcher_frame *f = upb_dispatcher_reset(&d->dispatcher, closure, d->plan->handlers->msgs[0]); upb_status_clear(&d->status); diff --git a/upb/pb/decoder_x64.dasc b/upb/pb/decoder_x64.dasc index 807191b..fa984ef 100644 --- a/upb/pb/decoder_x64.dasc +++ b/upb/pb/decoder_x64.dasc @@ -308,7 +308,6 @@ static void upb_assert_notnull(void *addr) { assert(addr != NULL); } // Decodes the next val into ARG3, advances PTR. static void upb_decoderplan_jit_decodefield(upb_decoderplan *plan, - upb_mhandlers *m, uint8_t type, size_t tag_size) { // Decode the value into arg 3 for the callback. switch (type) { @@ -559,7 +558,7 @@ static void upb_decoderplan_jit_field(upb_decoderplan *plan, uint64_t tag, return; } - upb_decoderplan_jit_decodefield(plan, m, f->type, tag_size); + upb_decoderplan_jit_decodefield(plan, f->type, tag_size); upb_decoderplan_jit_callcb(plan, f); // Epilogue: load next tag, check for repeated field. @@ -142,7 +142,8 @@ bool upb_errno_is_wouldblock() { bool upb_posix_codetostr(int code, char *buf, size_t len) { if (strerror_r(code, buf, len) == -1) { if (errno == EINVAL) { - return snprintf(buf, len, "Invalid POSIX error number %d\n", code) >= len; + int n = snprintf(buf, len, "Invalid POSIX error number %d\n", code); + return n >= (int)len; } else if (errno == ERANGE) { return false; } @@ -262,7 +262,7 @@ void upb_status_copy(upb_status *to, const upb_status *from); extern upb_errorspace upb_posix_errorspace; void upb_status_fromerrno(upb_status *status); -bool upb_errno_is_wouldblock(); +bool upb_errno_is_wouldblock(void); // Like vasprintf (which allocates a string large enough for the result), but // uses *buf (which can be NULL) as a starting point and reallocates it only if |