From 8ef6873e0e14309a1715a252a650bab0ae1a33ef Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Sun, 20 Mar 2011 13:13:51 -0700 Subject: upb_stream: all callbacks registered ahead-of-time. This is a significant change to the upb_stream protocol, and should hopefully be the last significant change. All callbacks are now registered ahead-of-time instead of having delegated callbacks registered at runtime, which makes it much easier to aggressively optimize ahead-of-time (like with a JIT). Other impacts of this change: - You no longer need to have loaded descriptor.proto as a upb_def to load other descriptors! This means the special-case code we used for bootstrapping is no longer necessary, and we no longer need to link the descriptor for descriptor.proto into upb. - A client can now register any upb_value as what will be delivered to their value callback, not just a upb_fielddef*. This should allow for other clients to get more bang out of the streaming decoder. This change unfortunately causes a bit of a performance regression -- I think largely due to highly suboptimal code that GCC generates when structs are returned by value. See: http://blog.reverberate.org/2011/03/19/when-a-compilers-slow-code-actually-bites-you/ On the other hand, once we have a JIT this should no longer matter. Performance numbers: plain.parsestream_googlemessage1.upb_table: 374 -> 396 (5.88) plain.parsestream_googlemessage2.upb_table: 616 -> 449 (-27.11) plain.parsetostruct_googlemessage1.upb_table_byref: 268 -> 269 (0.37) plain.parsetostruct_googlemessage1.upb_table_byval: 215 -> 204 (-5.12) plain.parsetostruct_googlemessage2.upb_table_byref: 307 -> 281 (-8.47) plain.parsetostruct_googlemessage2.upb_table_byval: 297 -> 272 (-8.42) omitfp.parsestream_googlemessage1.upb_table: 423 -> 410 (-3.07) omitfp.parsestream_googlemessage2.upb_table: 679 -> 483 (-28.87) omitfp.parsetostruct_googlemessage1.upb_table_byref: 287 -> 282 (-1.74) omitfp.parsetostruct_googlemessage1.upb_table_byval: 226 -> 219 (-3.10) omitfp.parsetostruct_googlemessage2.upb_table_byref: 315 -> 298 (-5.40) omitfp.parsetostruct_googlemessage2.upb_table_byval: 297 -> 287 (-3.37) --- benchmarks/parsestream.upb_table.c | 16 ++++++++-------- benchmarks/parsetostruct.upb_table.c | 19 +++++-------------- 2 files changed, 13 insertions(+), 22 deletions(-) (limited to 'benchmarks') diff --git a/benchmarks/parsestream.upb_table.c b/benchmarks/parsestream.upb_table.c index 85c9ff9..89649e6 100644 --- a/benchmarks/parsestream.upb_table.c +++ b/benchmarks/parsestream.upb_table.c @@ -47,11 +47,12 @@ static bool initialize() fprintf(stderr, "Error reading " MESSAGE_FILE "\n"); return false; } - upb_decoder_init(&decoder, def); + + upb_handlers_init(&handlers, def); + // Cause all messages to be read, but do nothing when they are. + upb_register_all(&handlers, NULL, NULL, NULL, NULL, NULL, NULL); + upb_decoder_init(&decoder, &handlers); upb_stringsrc_init(&stringsrc); - upb_handlers_init(&handlers); - static upb_handlerset handlerset = {}; // Empty handlers. - upb_register_handlerset(&handlers, &handlerset); return true; } @@ -61,6 +62,7 @@ static void cleanup() upb_def_unref(UPB_UPCAST(def)); upb_decoder_uninit(&decoder); upb_stringsrc_uninit(&stringsrc); + upb_handlers_uninit(&handlers); } static size_t run(int i) @@ -68,10 +70,8 @@ static size_t run(int i) (void)i; upb_status status = UPB_STATUS_INIT; upb_stringsrc_reset(&stringsrc, input_str); - upb_decoder_reset(&decoder, upb_stringsrc_bytesrc(&stringsrc)); - upb_src *src = upb_decoder_src(&decoder); - upb_src_sethandlers(src, &handlers); - upb_src_run(src, &status); + upb_decoder_reset(&decoder, upb_stringsrc_bytesrc(&stringsrc), NULL); + upb_decoder_decode(&decoder, &status); if(!upb_ok(&status)) goto err; return upb_string_len(input_str); diff --git a/benchmarks/parsetostruct.upb_table.c b/benchmarks/parsetostruct.upb_table.c index dfdd5b2..e778bc5 100644 --- a/benchmarks/parsetostruct.upb_table.c +++ b/benchmarks/parsetostruct.upb_table.c @@ -13,7 +13,6 @@ static upb_msg *msg; static upb_stringsrc strsrc; static upb_decoder d; static upb_handlers h; -static upb_msgpopulator p; static bool initialize() { @@ -54,9 +53,9 @@ static bool initialize() msg = upb_msg_new(def); upb_stringsrc_init(&strsrc); - upb_decoder_init(&d, def); - upb_msgpopulator_init(&p); - upb_handlers_init(&h); + upb_handlers_init(&h, def); + upb_msg_regdhandlers(&h); + upb_decoder_init(&d, &h); if (!BYREF) { // Pretend the input string is stack-allocated, which will force its data @@ -79,7 +78,6 @@ static void cleanup() upb_def_unref(UPB_UPCAST(def)); upb_stringsrc_uninit(&strsrc); upb_decoder_uninit(&d); - upb_msgpopulator_uninit(&p); upb_handlers_uninit(&h); } @@ -89,16 +87,9 @@ static size_t run(int i) upb_status status = UPB_STATUS_INIT; upb_msg_clear(msg, def); upb_stringsrc_reset(&strsrc, input_str); - upb_decoder_reset(&d, upb_stringsrc_bytesrc(&strsrc)); - upb_msgpopulator_reset(&p, msg, def); - upb_handlers_init(&h); - upb_msgpopulator_register_handlers(&p, &h); - upb_src *src = upb_decoder_src(&d); - upb_src_sethandlers(src, &h); - - upb_src_run(src, &status); + upb_decoder_reset(&d, upb_stringsrc_bytesrc(&strsrc), msg); + upb_decoder_decode(&d, &status); if(!upb_ok(&status)) goto err; - upb_status_uninit(&status); return upb_string_len(input_str); err: -- cgit v1.2.3