From a9570b0a5d361f39527ab034c4005c3927ed2df0 Mon Sep 17 00:00:00 2001 From: lwalkin Date: Fri, 10 Oct 2014 22:18:24 +0000 Subject: [PATCH 1/6] use consume_timeslice as intended --- c_src/decoder.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/c_src/decoder.c b/c_src/decoder.c index ad3e8d3..b37e056 100644 --- a/c_src/decoder.c +++ b/c_src/decoder.c @@ -754,8 +754,14 @@ decode_iter(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) start = d->i; while(d->i < bin.size) { //fprintf(stderr, "state: %d\r\n", dec_curr(d)); - if(should_yield(d->i - start, d->bytes_per_iter)) { - consume_timeslice(env, d->i - start, d->bytes_per_iter); + if(should_yield(d->i - start, d->bytes_per_iter) + /* A system could handle roughly 100kb per millisecond on a single core. + * So the total amount of work per millisecond is 100kb. + * We report the percentage of the time every (bytes_per_iter) bytes + * in hope that the system will ask as to yield. We don't yield until + * asked by the system according to our feedback (of questionable accuracy). + */ + && consume_timeslice(env, d->i - start, 100000)) { return enif_make_tuple5( env, st->atom_iter, From 8aa68b9a37cb8a2d8aa16fe15150ca81bc1289e7 Mon Sep 17 00:00:00 2001 From: lwalkin Date: Fri, 10 Oct 2014 22:18:09 +0000 Subject: [PATCH 2/6] should_yield() function should be inlined --- c_src/jiffy.h | 13 ++++++++++++- c_src/util.c | 10 ---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/c_src/jiffy.h b/c_src/jiffy.h index 6d869a5..6a7d73a 100644 --- a/c_src/jiffy.h +++ b/c_src/jiffy.h @@ -8,10 +8,22 @@ #define DEFAULT_BYTES_PER_ITER 2048 +#ifndef UNUSED +#define UNUSED __attribute__((unused)) +#endif + #define MAP_TYPE_PRESENT \ ((ERL_NIF_MAJOR_VERSION == 2 && ERL_NIF_MINOR_VERSION >= 6) \ || (ERL_NIF_MAJOR_VERSION > 2)) +static int UNUSED should_yield(size_t used, size_t limit) { + if(limit == 0 || used < limit) { + return 0; + } + + return 1; +} + typedef struct { ERL_NIF_TERM atom_ok; ERL_NIF_TERM atom_error; @@ -44,7 +56,6 @@ ERL_NIF_TERM make_error(jiffy_st* st, ErlNifEnv* env, const char* error); ERL_NIF_TERM make_obj_error(jiffy_st* st, ErlNifEnv* env, const char* error, ERL_NIF_TERM obj); int get_bytes_per_iter(ErlNifEnv* env, ERL_NIF_TERM val, size_t* bpi); -int should_yield(size_t used, size_t limit); int consume_timeslice(ErlNifEnv* env, size_t used, size_t limit); ERL_NIF_TERM decode_init(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]); diff --git a/c_src/util.c b/c_src/util.c index c30a6d7..f9f3bfc 100644 --- a/c_src/util.c +++ b/c_src/util.c @@ -62,16 +62,6 @@ get_bytes_per_iter(ErlNifEnv* env, ERL_NIF_TERM val, size_t* bpi) return 1; } -int -should_yield(size_t used, size_t limit) -{ - if(limit == 0 || used < limit) { - return 0; - } - - return 1; -} - int consume_timeslice(ErlNifEnv* env, size_t used, size_t limit) { From 42f2c7e105ad687047d29f5123b966bbe8638d6d Mon Sep 17 00:00:00 2001 From: lwalkin Date: Fri, 10 Oct 2014 22:51:23 +0000 Subject: [PATCH 3/6] recognize [with_trailer] option, but do not use yet --- c_src/decoder.c | 4 ++++ c_src/jiffy.c | 1 + c_src/jiffy.h | 1 + src/jiffy.erl | 12 +++++++++++- test/jiffy_15_trailer_tests.erl | 11 +++++++++++ 5 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 test/jiffy_15_trailer_tests.erl diff --git a/c_src/decoder.c b/c_src/decoder.c index b37e056..b71f55d 100644 --- a/c_src/decoder.c +++ b/c_src/decoder.c @@ -53,6 +53,7 @@ typedef struct { int is_partial; int return_maps; int use_nil; + int with_trailer; char* p; unsigned char* u; @@ -81,6 +82,7 @@ dec_new(ErlNifEnv* env) d->is_partial = 0; d->return_maps = 0; d->use_nil = 0; + d->with_trailer = 0; d->p = NULL; d->u = NULL; @@ -712,6 +714,8 @@ decode_init(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) #endif } else if(enif_compare(val, d->atoms->atom_use_nil) == 0) { d->use_nil = 1; + } else if(enif_compare(val, d->atoms->atom_with_trailer) == 0) { + d->with_trailer = 1; } else { return enif_make_badarg(env); } diff --git a/c_src/jiffy.c b/c_src/jiffy.c index 31aa854..544fc5c 100644 --- a/c_src/jiffy.c +++ b/c_src/jiffy.c @@ -28,6 +28,7 @@ load(ErlNifEnv* env, void** priv, ERL_NIF_TERM info) st->atom_return_maps = make_atom(env, "return_maps"); st->atom_nil = make_atom(env, "nil"); st->atom_use_nil = make_atom(env, "use_nil"); + st->atom_with_trailer = make_atom(env, "with_trailer"); // Markers used in encoding st->ref_object = make_atom(env, "$object_ref$"); diff --git a/c_src/jiffy.h b/c_src/jiffy.h index 6a7d73a..d1d607c 100644 --- a/c_src/jiffy.h +++ b/c_src/jiffy.h @@ -42,6 +42,7 @@ typedef struct { ERL_NIF_TERM atom_return_maps; ERL_NIF_TERM atom_nil; ERL_NIF_TERM atom_use_nil; + ERL_NIF_TERM atom_with_trailer; ERL_NIF_TERM ref_object; ERL_NIF_TERM ref_array; diff --git a/src/jiffy.erl b/src/jiffy.erl index 23a0c9d..7e5d34e 100644 --- a/src/jiffy.erl +++ b/src/jiffy.erl @@ -19,13 +19,23 @@ decode(Data, Opts) when is_binary(Data), is_list(Opts) -> {partial, EJson} -> finish_decode(EJson); {iter, Decoder, Val, Objs, Curr} -> - decode_loop(Data, Decoder, Val, Objs, Curr); + trailer_or_error(decode_loop(Data, Decoder, Val, Objs, Curr), Opts, Data); EJson -> EJson end; decode(Data, Opts) when is_list(Data) -> decode(iolist_to_binary(Data), Opts). +% Only return trailing data if explicitly requested in the options. +trailer_or_error({with_trailer, _EJson, TrailerData} = WithTrailer, Opts, Data) -> + case lists:member(with_trailer, Opts) of + true -> WithTrailer; + false -> + AtByte = iolist_size(Data) - iolist_size(TrailerData) + 1, + throw({error,{AtByte,invalid_trailing_data}}) + end; +trailer_or_error(EJson, _Opts, _Data) -> EJson. + encode(Data) -> encode(Data, []). diff --git a/test/jiffy_15_trailer_tests.erl b/test/jiffy_15_trailer_tests.erl new file mode 100644 index 0000000..9832e89 --- /dev/null +++ b/test/jiffy_15_trailer_tests.erl @@ -0,0 +1,11 @@ +% This file is part of Jiffy released under the MIT license. +% See the LICENSE file for more information. + +-module(jiffy_15_trailer_tests). + +-include_lib("eunit/include/eunit.hrl"). + +trailer_test_() -> + {"trailer", [ + ?_assertEqual(true, jiffy:decode(<<"true">>, [with_trailer])) + ]}. From 661b23acf4c41c19480df934902660fa969d52b8 Mon Sep 17 00:00:00 2001 From: lwalkin Date: Fri, 10 Oct 2014 23:15:54 +0000 Subject: [PATCH 4/6] Implemented with_trailer support --- c_src/decoder.c | 17 ++++++++++++++--- test/jiffy_15_trailer_tests.erl | 7 ++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/c_src/decoder.c b/c_src/decoder.c index b71f55d..066fcc1 100644 --- a/c_src/decoder.c +++ b/c_src/decoder.c @@ -730,6 +730,7 @@ decode_iter(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) Decoder* d; jiffy_st* st = (jiffy_st*) enif_priv_data(env); + ERL_NIF_TERM bin_term = argv[0]; ErlNifBinary bin; ERL_NIF_TERM objs; @@ -740,7 +741,7 @@ decode_iter(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) if(argc != 5) { return enif_make_badarg(env); - } else if(!enif_inspect_binary(env, argv[0], &bin)) { + } else if(!enif_inspect_binary(env, bin_term, &bin)) { return enif_make_badarg(env); } else if(!enif_get_resource(env, argv[1], st->res_dec, (void**) &d)) { return enif_make_badarg(env); @@ -1038,8 +1039,16 @@ decode_iter(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) d->i++; break; default: - ret = dec_error(d, "invalid_trailing_data"); - goto done; + if(d->with_trailer) { + ERL_NIF_TERM trailer = enif_make_sub_binary(env, + bin_term, d->i, bin.size - d->i); + val = enif_make_tuple3(env, d->atoms->atom_with_trailer, + val, trailer); + goto soft_done; + } else { + ret = dec_error(d, "invalid_trailing_data"); + goto done; + } } break; @@ -1049,6 +1058,8 @@ decode_iter(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) } } +soft_done: + if(dec_curr(d) != st_done) { ret = dec_error(d, "truncated_json"); } else if(d->is_partial) { diff --git a/test/jiffy_15_trailer_tests.erl b/test/jiffy_15_trailer_tests.erl index 9832e89..1190011 100644 --- a/test/jiffy_15_trailer_tests.erl +++ b/test/jiffy_15_trailer_tests.erl @@ -6,6 +6,11 @@ -include_lib("eunit/include/eunit.hrl"). trailer_test_() -> + Opts = [with_trailer], {"trailer", [ - ?_assertEqual(true, jiffy:decode(<<"true">>, [with_trailer])) + ?_assertEqual(true, jiffy:decode(<<"true">>, Opts)), + ?_assertMatch({with_trailer, true, <<";">>}, jiffy:decode(<<"true;">>, Opts)), + ?_assertMatch({with_trailer, true, <<"[]">>}, jiffy:decode(<<"true[]">>, Opts)), + ?_assertMatch({with_trailer, [], <<"{}">>}, jiffy:decode(<<"[]{}">>, Opts)), + ?_assertMatch({with_trailer, 1, <<"2 3">>}, jiffy:decode(<<"1 2 3">>, Opts)) ]}. From 4480ca3558be7758850486e14d6286bb59fdba3a Mon Sep 17 00:00:00 2001 From: lwalkin Date: Fri, 10 Oct 2014 23:18:06 +0000 Subject: [PATCH 5/6] documentation for with_trailer option --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index a609b77..174f660 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,10 @@ The options for decode are: * `return_maps` - Tell Jiffy to return objects using the maps data type on VMs that support it. This raises an error on VMs that don't support maps. +* `with_trailer` - Tell Jiffy to return the trailing unparsed data (if any) along with + the parsed term instead of failing with {error,{_,invalid_traling_data}}. When + the trailer is available, the return value is {with_trailer, EJson, Trailer}, + where Trailer is a sub-binary of the input, for efficiency. `jiffy:encode/1,2` ------------------ From 9e10787013d9639b44560959f8f69785e11a138b Mon Sep 17 00:00:00 2001 From: lwalkin Date: Mon, 13 Oct 2014 23:40:55 +0000 Subject: [PATCH 6/6] trailer_or_error is not required for correctness --- src/jiffy.erl | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/jiffy.erl b/src/jiffy.erl index 7e5d34e..23a0c9d 100644 --- a/src/jiffy.erl +++ b/src/jiffy.erl @@ -19,23 +19,13 @@ decode(Data, Opts) when is_binary(Data), is_list(Opts) -> {partial, EJson} -> finish_decode(EJson); {iter, Decoder, Val, Objs, Curr} -> - trailer_or_error(decode_loop(Data, Decoder, Val, Objs, Curr), Opts, Data); + decode_loop(Data, Decoder, Val, Objs, Curr); EJson -> EJson end; decode(Data, Opts) when is_list(Data) -> decode(iolist_to_binary(Data), Opts). -% Only return trailing data if explicitly requested in the options. -trailer_or_error({with_trailer, _EJson, TrailerData} = WithTrailer, Opts, Data) -> - case lists:member(with_trailer, Opts) of - true -> WithTrailer; - false -> - AtByte = iolist_size(Data) - iolist_size(TrailerData) + 1, - throw({error,{AtByte,invalid_trailing_data}}) - end; -trailer_or_error(EJson, _Opts, _Data) -> EJson. - encode(Data) -> encode(Data, []).