From 70293f5faabe08fc9bdef26ad0b1d6afe19e8f79 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 11 May 2013 17:56:13 -0700 Subject: [PATCH 1/2] Open source fixes: builds on OS X again. --- Makefile | 10 ++++++---- upb/def.c | 1 + upb/google/bridge.cc | 2 +- upb/google/proto2.cc | 4 ++-- upb/handlers.h | 28 ++++++++++++++-------------- upb/pb/decoder.c | 1 - upb/table.c | 4 ---- 7 files changed, 24 insertions(+), 26 deletions(-) diff --git a/Makefile b/Makefile index c5df79915a..787b2cddd5 100644 --- a/Makefile +++ b/Makefile @@ -49,7 +49,7 @@ CXX=g++ CFLAGS=-std=gnu99 CXXFLAGS=-Ibindings/cpp INCLUDE=-Itests -I. -CPPFLAGS=$(INCLUDE) -Wall -Wextra $(USER_CFLAGS) +CPPFLAGS=$(INCLUDE) -Wall -Wextra -Wno-sign-compare $(USER_CFLAGS) LDLIBS=-lpthread upb/libupb.a LUA=lua5.1 # 5.1 and 5.2 should both be supported @@ -82,7 +82,7 @@ deps: Makefile $(ALLSRC) # The core library. CORE= \ - upb/bytestream.c \ + upb/bytestream.upb.c \ upb/def.c \ upb/descriptor/reader.c \ upb/descriptor/descriptor.upb.c \ @@ -101,9 +101,10 @@ CORE= \ PB= \ upb/pb/decoder.c \ upb/pb/glue.c \ - upb/pb/textprinter.c \ upb/pb/varint.c \ + #upb/pb/textprinter.c \ + # Rules. ####################################################################### @@ -247,7 +248,8 @@ $(SIMPLE_CXX_TESTS): % : %.cc $(E) CXX $< $(Q) $(CXX) $(CXXFLAGS) $(CPPFLAGS) -o $@ tests/testmain.o $< $(LIBUPB) -VALGRIND=valgrind --leak-check=full --error-exitcode=1 +#VALGRIND=valgrind --leak-check=full --error-exitcode=1 --track-origins=yes +VALGRIND= test: tests @echo Running all tests under valgrind. @set -e # Abort on error. diff --git a/upb/def.c b/upb/def.c index aad0e51a90..b8d32594ac 100644 --- a/upb/def.c +++ b/upb/def.c @@ -676,6 +676,7 @@ upb_descriptortype_t upb_fielddef_descriptortype(const upb_fielddef *f) { return upb_fielddef_istagdelim(f) ? UPB_DESCRIPTOR_TYPE_GROUP : UPB_DESCRIPTOR_TYPE_MESSAGE; } + return 0; } bool upb_fielddef_setlabel(upb_fielddef *f, upb_label_t label) { diff --git a/upb/google/bridge.cc b/upb/google/bridge.cc index f5b664cfe5..93e5c27b44 100644 --- a/upb/google/bridge.cc +++ b/upb/google/bridge.cc @@ -208,7 +208,7 @@ static upb::MessageDef* NewMessageDef(const goog::Message& m, const void* owner, // Must do this before processing submessages to prevent infinite recursion. defs->AddMessage(&m, md); - vector fields; + std::vector fields; d->file()->pool()->FindAllExtensions(d, &fields); for (int i = 0; i < d->field_count(); i++) { fields.push_back(d->field(i)); diff --git a/upb/google/proto2.cc b/upb/google/proto2.cc index d7ad919aff..8f9bcd8643 100644 --- a/upb/google/proto2.cc +++ b/upb/google/proto2.cc @@ -409,7 +409,7 @@ case goog::FieldDescriptor::cpptype: \ static_cast(frame->handler_data()); if (data->IsValidValue(val)) { goog::RepeatedField* r = - data->GetFieldPointer>(m); + data->GetFieldPointer >(m); r->Add(val); } else { data->mutable_unknown_fields(m)->AddVarint(data->field_number(), val); @@ -487,7 +487,7 @@ case goog::FieldDescriptor::cpptype: \ } else { StringHandlerData* data = new StringHandlerData(proto2_f, r); h->SetStartStringHandler(f, &StartString, data, - &upb::DeletePointer>); + &upb::DeletePointer >); } } diff --git a/upb/handlers.h b/upb/handlers.h index 582ba43685..79ca0d8983 100644 --- a/upb/handlers.h +++ b/upb/handlers.h @@ -915,37 +915,37 @@ inline bool Handlers::SetUInt64Handler( } #endif -#define SET_VALUE_HANDLER(type, ctype) \ +#define SET_VALUE_HANDLER(type, ctype, handlertype) \ template<> \ inline bool Handlers::SetValueHandler( \ const FieldDef* f, \ - typename Handlers::Value::Handler* handler, \ + handlertype* handler, \ void* data, Handlers::Free* cleanup) { \ return upb_handlers_set ## type(this, f, handler, data, cleanup); \ } \ template<> \ inline bool Handlers::SetValueHandler( \ const char* f, \ - typename Handlers::Value::Handler* handler, \ + handlertype* handler, \ void* data, Handlers::Free* cleanup) { \ return upb_handlers_set ## type ## _n(this, f, handler, data, cleanup); \ } -SET_VALUE_HANDLER(double, double); -SET_VALUE_HANDLER(float, float); -SET_VALUE_HANDLER(uint64, upb_uint64_t); -SET_VALUE_HANDLER(uint32, upb_uint32_t); -SET_VALUE_HANDLER(int64, upb_int64_t); -SET_VALUE_HANDLER(int32, upb_int32_t); -SET_VALUE_HANDLER(bool, bool); +SET_VALUE_HANDLER(double, double, DoubleHandler); +SET_VALUE_HANDLER(float, float, FloatHandler); +SET_VALUE_HANDLER(uint64, upb_uint64_t, UInt64Handler); +SET_VALUE_HANDLER(uint32, upb_uint32_t, UInt32Handler); +SET_VALUE_HANDLER(int64, upb_int64_t, Int64Handler); +SET_VALUE_HANDLER(int32, upb_int32_t, Int32Handler); +SET_VALUE_HANDLER(bool, bool, BoolHandler); #ifdef UPB_TWO_32BIT_TYPES -SET_VALUE_HANDLER(int32alt, upb_int32alt_t); -SET_VALUE_HANDLER(uint32alt, upb_uint32alt_t); +SET_VALUE_HANDLER(int32alt, upb_int32alt_t, Int32Handler2); +SET_VALUE_HANDLER(uint32alt, upb_uint32alt_t, UInt32Handler2); #endif #ifdef UPB_TWO_64BIT_TYPES -SET_VALUE_HANDLER(int64alt, upb_int64alt_t); -SET_VALUE_HANDLER(uint64alt, upb_uint64alt_t); +SET_VALUE_HANDLER(int64alt, upb_int64alt_t, Int64Handler2); +SET_VALUE_HANDLER(uint64alt, upb_uint64alt_t, UInt64Handler2); #endif #undef SET_VALUE_HANDLER diff --git a/upb/pb/decoder.c b/upb/pb/decoder.c index 2bfc71713b..da4c48d205 100644 --- a/upb/pb/decoder.c +++ b/upb/pb/decoder.c @@ -290,7 +290,6 @@ static void suspendjmp(upb_pbdecoder *d) { } static void advancetobuf(upb_pbdecoder *d, const char *buf, size_t len) { - assert(len >= 0); assert(d->ptr == d->end); d->bufstart_ofs += (d->ptr - d->buf); switchtobuf(d, buf, buf + len); diff --git a/upb/table.c b/upb/table.c index a54e7151d1..40f841d501 100644 --- a/upb/table.c +++ b/upb/table.c @@ -22,12 +22,8 @@ static const double MAX_LOAD = 0.85; static const double MIN_DENSITY = 0.1; int upb_log2(uint64_t v) { -#ifdef __GNUC__ - int ret = 31 - __builtin_clz(v); -#else int ret = 0; while (v >>= 1) ret++; -#endif return UPB_MIN(UPB_MAXARRSIZE, ret); } From 622481990b17bed5e6fd69a1cab5d5f20413be79 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 11 May 2013 18:48:23 -0700 Subject: [PATCH 2/2] Updated benchmarks to new APIs. --- Makefile | 4 +- benchmarks/parsestream.upb.c | 75 ++++++++++++++++----------------- benchmarks/parsetoproto2.upb.cc | 48 ++++++++++----------- 3 files changed, 61 insertions(+), 66 deletions(-) diff --git a/Makefile b/Makefile index 787b2cddd5..bd4fda9717 100644 --- a/Makefile +++ b/Makefile @@ -51,7 +51,7 @@ CXXFLAGS=-Ibindings/cpp INCLUDE=-Itests -I. CPPFLAGS=$(INCLUDE) -Wall -Wextra -Wno-sign-compare $(USER_CFLAGS) LDLIBS=-lpthread upb/libupb.a -LUA=lua5.1 # 5.1 and 5.2 should both be supported +LUA=lua # 5.1 and 5.2 should both be supported # Build with "make Q=" to see all commands that are being executed. Q=@ @@ -173,7 +173,7 @@ upb/def.lo: upb/def.c upb/pb/decoder_x64.h: upb/pb/decoder_x64.dasc $(E) DYNASM $< - $(Q) $(LUA) dynasm/dynasm.lua upb/pb/decoder_x64.dasc > upb/pb/decoder_x64.h + $(Q) $(LUA) dynasm/dynasm.lua upb/pb/decoder_x64.dasc > upb/pb/decoder_x64.h || (rm upb/pb/decoder_x64.h ; false) ifneq ($(shell uname), Darwin) upb/pb/jit_debug_elf_file.o: upb/pb/jit_debug_elf_file.s diff --git a/benchmarks/parsestream.upb.c b/benchmarks/parsestream.upb.c index b5a353c17b..0d709a4949 100644 --- a/benchmarks/parsestream.upb.c +++ b/benchmarks/parsestream.upb.c @@ -10,33 +10,31 @@ static char *input_str; static size_t input_len; static const upb_msgdef *def; -static upb_decoder decoder; -static upb_stringsrc stringsrc; -static upb_decoderplan *plan; +upb_pipeline pipeline; +static upb_sink *sink; -static upb_sflow_t startsubmsg(void *_m, upb_value fval) { - (void)_m; - (void)fval; - return UPB_CONTINUE_WITH(NULL); +static void *startsubmsg(const upb_sinkframe *frame) { + UPB_UNUSED(frame); + return input_str; } -static upb_flow_t value(void *closure, upb_value fval, upb_value val) { - (void)closure; - (void)fval; - (void)val; - return UPB_CONTINUE; -} - -void onfreg(void *c, upb_fhandlers *fh, const upb_fielddef *f) { - upb_fhandlers_setvalue(fh, &value); - upb_fhandlers_setstartsubmsg(fh, &startsubmsg); +void onmreg(void *c, upb_handlers *h) { + upb_msg_iter i; + upb_msg_begin(&i, upb_handlers_msgdef(h)); + for(; !upb_msg_done(&i); upb_msg_next(&i)) { + const upb_fielddef *f = upb_msg_iter_field(&i); + if (upb_fielddef_type(f) == UPB_TYPE_MESSAGE) { + upb_handlers_setstartsubmsg(h, f, startsubmsg, NULL, NULL); + } + } + UPB_UNUSED(c); } static bool initialize() { // Initialize upb state, decode descriptor. upb_status status = UPB_STATUS_INIT; - upb_symtab *s = upb_symtab_new(); + upb_symtab *s = upb_symtab_new(&s); upb_load_descriptor_file_into_symtab(s, MESSAGE_DESCRIPTOR_FILE, &status); if(!upb_ok(&status)) { fprintf(stderr, "Error reading descriptor: %s\n", @@ -44,12 +42,12 @@ static bool initialize() return false; } - def = upb_dyncast_msgdef_const(upb_symtab_lookup(s, MESSAGE_NAME, &def)); + def = upb_dyncast_msgdef(upb_symtab_lookup(s, MESSAGE_NAME, &def)); if(!def) { fprintf(stderr, "Error finding symbol '%s'.\n", MESSAGE_NAME); return false; } - upb_symtab_unref(s); + upb_symtab_unref(s, &s); // Read the message data itself. input_str = upb_readfile(MESSAGE_FILE, &input_len); @@ -58,36 +56,37 @@ static bool initialize() return false; } - upb_handlers *handlers = upb_handlers_new(); // Cause all messages to be read, but do nothing when they are. - upb_handlers_regmsgdef(handlers, def, NULL, &upb_onfreg_hset, NULL); - upb_decoder_init(&decoder); - plan = upb_decoderplan_new(handlers, JIT); - upb_decoder_resetplan(&decoder, plan, 0); - upb_handlers_unref(handlers); - upb_stringsrc_init(&stringsrc); + const upb_handlers* handlers = + upb_handlers_newfrozen(def, NULL, &handlers, &onmreg, NULL); + const upb_handlers* decoder_handlers = + upb_pbdecoder_gethandlers(handlers, JIT, &decoder_handlers); + upb_msgdef_unref(def, &def); + + upb_pipeline_init(&pipeline, NULL, 0, upb_realloc, NULL); + upb_sink *s2 = upb_pipeline_newsink(&pipeline, handlers); + sink = upb_pipeline_newsink(&pipeline, decoder_handlers); + upb_pipeline_donateref(&pipeline, decoder_handlers, &decoder_handlers); + upb_pipeline_donateref(&pipeline, handlers, &handlers); + upb_pbdecoder *decoder = upb_sinkframe_userdata(upb_sink_top(sink)); + upb_pbdecoder_resetsink(decoder, s2); return true; } static void cleanup() { free(input_str); - upb_def_unref(UPB_UPCAST(def), &def); - upb_decoder_uninit(&decoder); - upb_decoderplan_unref(plan); - upb_stringsrc_uninit(&stringsrc); + upb_pipeline_uninit(&pipeline); } static size_t run(int i) { (void)i; - upb_status status = UPB_STATUS_INIT; - upb_stringsrc_reset(&stringsrc, input_str, input_len); - upb_decoder_resetinput(&decoder, upb_stringsrc_allbytes(&stringsrc), NULL); - if (upb_decoder_decode(&decoder) != UPB_OK) goto err; + upb_pipeline_reset(&pipeline); + if (!upb_bytestream_putstr(sink, input_str, input_len)) { + fprintf(stderr, "Decode error: %s", upb_status_getstr(upb_pipeline_status(&pipeline))); + return 0; + } return input_len; -err: - fprintf(stderr, "Decode error: %s", upb_status_getstr(&status)); - return 0; } diff --git a/benchmarks/parsetoproto2.upb.cc b/benchmarks/parsetoproto2.upb.cc index 5023b0e882..4ad1faa8a6 100644 --- a/benchmarks/parsetoproto2.upb.cc +++ b/benchmarks/parsetoproto2.upb.cc @@ -4,22 +4,19 @@ #include "main.c" #include -#include "upb/bytestream.hpp" -#include "upb/def.hpp" -#include "upb/msg.hpp" -#include "upb/pb/decoder.hpp" +#include "upb/bytestream.h" +#include "upb/def.h" +#include "upb/pb/decoder.h" #include "upb/pb/glue.h" -#include "upb/proto2_bridge.hpp" +#include "upb/google/bridge.h" #include MESSAGE_HFILE const char *str; size_t len; MESSAGE_CIDENT msg[NUM_MESSAGES]; -MESSAGE_CIDENT msg2; -upb::StringSource strsrc; -upb::Decoder d; -const upb::MessageDef *def; -upb::DecoderPlan* plan; +upb::SeededPipeline<8192> pipeline(upb_realloc, NULL); +upb::Sink *decoder_sink; +upb::Sink *proto2_sink; static bool initialize() { @@ -30,32 +27,31 @@ static bool initialize() return false; } - def = upb::proto2_bridge::NewFinalMessageDef(msg2, &def); + const upb::Handlers* h = upb::google::NewWriteHandlers(MESSAGE_CIDENT(), &h); + const upb::Handlers* h2 = upb::pb::GetDecoderHandlers(h, JIT, &h2); - msg2.ParseFromArray(str, len); + proto2_sink = pipeline.NewSink(h); + decoder_sink = pipeline.NewSink(h2); + pipeline.DonateRef(h, &h); + pipeline.DonateRef(h2, &h2); - upb::Handlers* h = upb::Handlers::New(); - upb::RegisterWriteHandlers(h, def); - plan = upb::DecoderPlan::New(h, JIT); - d.ResetPlan(plan, 0); - h->Unref(); + upb::pb::Decoder* d = decoder_sink->top()->GetUserdata(); + upb::pb::ResetDecoderSink(d, proto2_sink); return true; } static void cleanup() { - def->Unref(&def); - plan->Unref(); } static size_t run(int i) { + pipeline.Reset(); + proto2_sink->Reset(&msg[i % NUM_MESSAGES]); msg[i % NUM_MESSAGES].Clear(); - strsrc.Reset(str, len); - d.ResetInput(strsrc.AllBytes(), &msg[i % NUM_MESSAGES]); - if (d.Decode() != UPB_OK) goto err; - return len; -err: - fprintf(stderr, "Decode error: %s", d.status().GetString()); - return 0; + if (!upb::PutStringToBytestream(decoder_sink, str, len)) { + fprintf(stderr, "Decode error: %s", pipeline.status().GetString()); + return 0; + } + return len; }