Skip to content

Commit

Permalink
wasm: remove opa_number_float (open-policy-agent#3414)
Browse files Browse the repository at this point in the history 
This is the bare minimum to address open-policy-agent#3298.

Little change in the benchmarks, there's much potential in improving our
number handling, but that's something we should do purposefully later.

name                                    old time/op  new time/op  delta
WASMArrayIteration/10-16                 420µs ± 2%   404µs ± 2%   -3.82%  (p=0.008 n=5+5)
WASMArrayIteration/100-16                435µs ± 6%   410µs ± 4%     ~     (p=0.056 n=5+5)
WASMArrayIteration/1000-16               582µs ± 2%   554µs ± 4%   -4.86%  (p=0.016 n=5+5)
WASMArrayIteration/10000-16             2.22ms ± 4%  2.23ms ± 6%     ~     (p=0.690 n=5+5)
WASMSetIteration/10-16                   426µs ± 6%   401µs ± 4%   -5.84%  (p=0.032 n=5+5)
WASMSetIteration/100-16                  442µs ± 3%   427µs ± 3%   -3.45%  (p=0.032 n=5+5)
WASMSetIteration/1000-16                 659µs ± 2%   673µs ± 2%     ~     (p=0.151 n=5+5)
WASMSetIteration/10000-16               3.86ms ± 7%  3.77ms ± 5%     ~     (p=0.310 n=5+5)
WASMObjectIteration/10-16                428µs ± 3%   426µs ± 5%     ~     (p=1.000 n=5+5)
WASMObjectIteration/100-16               433µs ± 6%   447µs ± 4%     ~     (p=0.222 n=5+5)
WASMObjectIteration/1000-16              687µs ± 3%   702µs ± 3%     ~     (p=0.151 n=5+5)
WASMObjectIteration/10000-16            3.88ms ± 3%  4.13ms ± 6%   +6.42%  (p=0.032 n=5+5)
WASMLargeJSON/10x10-16                   461µs ± 5%   448µs ± 4%     ~     (p=0.421 n=5+5)
WASMLargeJSON/10x100-16                  484µs ± 3%   497µs ± 9%     ~     (p=0.548 n=5+5)
WASMLargeJSON/10x1000-16                 831µs ± 4%   917µs ± 2%  +10.29%  (p=0.008 n=5+5)
WASMLargeJSON/10x10000-16               5.63ms ± 3%  6.12ms ± 5%   +8.67%  (p=0.008 n=5+5)
WASMLargeJSON/100x100-16                 688µs ± 2%   730µs ± 3%   +6.17%  (p=0.008 n=5+5)
WASMLargeJSON/100x1000-16               3.20ms ±10%  3.27ms ± 5%     ~     (p=0.548 n=5+5)
WASMVirtualDocs/total=1/hit=1-16         421µs ± 2%   410µs ± 3%     ~     (p=0.095 n=5+5)
WASMVirtualDocs/total=10/hit=1-16        422µs ± 3%   406µs ± 5%     ~     (p=0.095 n=5+5)
WASMVirtualDocs/total=100/hit=1-16       430µs ± 5%   420µs ± 4%     ~     (p=0.548 n=5+5)
WASMVirtualDocs/total=1000/hit=1-16      460µs ± 2%   457µs ± 5%     ~     (p=0.841 n=5+5)
WASMVirtualDocs/total=10/hit=10-16       413µs ± 6%   429µs ± 5%     ~     (p=0.056 n=5+5)
WASMVirtualDocs/total=100/hit=10-16      417µs ± 6%   436µs ± 2%     ~     (p=0.056 n=5+5)
WASMVirtualDocs/total=1000/hit=10-16     473µs ± 5%   480µs ± 2%     ~     (p=0.690 n=5+5)
WASMVirtualDocs/total=100/hit=100-16     453µs ± 2%   454µs ± 8%     ~     (p=0.421 n=5+5)
WASMVirtualDocs/total=1000/hit=100-16    503µs ± 4%   499µs ± 6%     ~     (p=1.000 n=5+5)
WASMVirtualDocs/total=1000/hit=1000-16   843µs ± 1%   823µs ± 5%     ~     (p=0.222 n=5+5)

Signed-off-by: Stephan Renatus <[email protected]>
  • Loading branch information
@srenatus
srenatus authored Apr 28, 2021
1 parent 0d0a5ac commit 8f91118
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 69 deletions.
13 changes: 4 additions & 9 deletions internal/compiler/wasm/opa/callgraph.csv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ __force_import_opa_builtins,opa_builtin4
opa_to_number,opa_value_type
opa_to_number,opa_number_int
opa_to_number,opa_atof64
opa_to_number,opa_number_float
opa_to_number,opa_number_ref
opa_base64_is_valid,opa_value_type
opa_base64_is_valid,base64_gen_decode
opa_base64_is_valid,free
Expand Down Expand Up @@ -288,13 +288,9 @@ opa_json_parse,opa_json_parse_token
opa_value_parse,opa_json_lex_read
opa_value_parse,opa_json_parse_token
opa_json_writer_emit_boolean,opa_malloc
opa_json_writer_emit_float,snprintf_
opa_json_writer_emit_float,opa_strlen
opa_json_writer_emit_float,opa_malloc
opa_json_writer_emit_integer,opa_itoa
opa_json_writer_emit_integer,opa_strlen
opa_json_writer_emit_integer,opa_malloc
opa_json_writer_emit_number,opa_json_writer_emit_float
opa_json_writer_emit_number,opa_json_writer_emit_integer
opa_json_writer_emit_number,opa_malloc
opa_json_writer_emit_number,opa_abort
Expand Down Expand Up @@ -348,14 +344,14 @@ opa_mpd_init,mpd_qset_i32
opa_mpd_init,opa_abort
opa_mpd_del,mpd_del
opa_number_to_bf,opa_value_type
opa_number_to_bf,snprintf_
opa_number_to_bf,opa_abort
opa_number_to_bf,mpd_qnew
opa_number_to_bf,mpd_qset_string
opa_number_to_bf,malloc
opa_number_to_bf,memcpy
opa_number_to_bf,mpd_qset_string
opa_number_to_bf,opa_abort
opa_number_to_bf,free
opa_number_to_bf,mpd_qset_i32
opa_number_to_bf,snprintf_
opa_bf_to_number,mpd_qget_i32
opa_bf_to_number,mpd_del
opa_bf_to_number,opa_number_int
Expand Down Expand Up @@ -743,7 +739,6 @@ __opa_object_grow,opa_value_hash
__opa_object_grow,opa_value_compare
__opa_object_grow,opa_free
opa_boolean,opa_malloc
opa_number_float,opa_malloc
opa_number_ref,opa_malloc
opa_number_int,opa_malloc
opa_string,opa_malloc
Expand Down
4 changes: 2 additions & 2 deletions wasm/src/conversions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ opa_value *opa_to_number(opa_value *v)
{
opa_string_t *a = opa_cast_string(v);
double n;

// NOTE: we're only using opa_atof64 for validation here
if (opa_atof64(a->v, a->len, &n) != 0)
{
return NULL;
}

return opa_number_float(n);
return opa_number_ref(a->v, a->len);
}
default:
return NULL;
Expand Down
9 changes: 0 additions & 9 deletions wasm/src/json.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -810,13 +810,6 @@ int opa_json_writer_emit_boolean(opa_json_writer *w, opa_boolean_t *b)
return opa_json_writer_emit_chars(w, bs, sizeof(bs)-1);
}

int opa_json_writer_emit_float(opa_json_writer *w, double f)
{
char str[32];
snprintf(str, sizeof(str), "%g", f);
return opa_json_writer_emit_chars(w, str, opa_strlen(str));
}

int opa_json_writer_emit_integer(opa_json_writer *w, long long i)
{
char str[sizeof(i)*8+1]; // once base=2 is supported we need 8 bits per byte.
Expand All @@ -828,8 +821,6 @@ int opa_json_writer_emit_number(opa_json_writer *w, opa_number_t *n)
{
switch (n->repr)
{
case OPA_NUMBER_REPR_FLOAT:
return opa_json_writer_emit_float(w, n->v.f);
case OPA_NUMBER_REPR_INT:
return opa_json_writer_emit_integer(w, n->v.i);
case OPA_NUMBER_REPR_REF:
Expand Down
13 changes: 0 additions & 13 deletions wasm/src/mpd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,6 @@ mpd_t *opa_number_to_bf(opa_value *v)

switch (n->repr)
{
case OPA_NUMBER_REPR_FLOAT:
{
char buf[32]; // PRINTF_FTOA_BUFFER_SIZE
if (snprintf(buf, sizeof(buf), "%f", n->v.f) == sizeof(buf))
{
opa_abort("opa_number_to_bf: overflow");
}

r = mpd_qnew();
mpd_qset_string(r, buf, mpd_default_ctx(), &status);
}
break;

case OPA_NUMBER_REPR_REF:
r = mpd_qnew();

Expand Down
24 changes: 0 additions & 24 deletions wasm/src/value.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,14 +319,6 @@ int opa_value_compare_number(opa_number_t *a, opa_number_t *b)
return 0;
}

if (a->repr == OPA_NUMBER_REPR_FLOAT && b->repr == OPA_NUMBER_REPR_FLOAT)
{
double da = opa_number_as_float(a);
double db = opa_number_as_float(b);

return opa_value_compare_float(da, db);
}

mpd_t *ba = opa_number_to_bf(&a->hdr);
mpd_t *bb = opa_number_to_bf(&b->hdr);

Expand Down Expand Up @@ -752,8 +744,6 @@ opa_value *opa_value_shallow_copy_number(opa_number_t *n)
{
switch (n->repr)
{
case OPA_NUMBER_REPR_FLOAT:
return opa_number_float(n->v.f);
case OPA_NUMBER_REPR_REF:
return opa_number_ref(n->v.ref.s, n->v.ref.len);
case OPA_NUMBER_REPR_INT:
Expand Down Expand Up @@ -916,16 +906,6 @@ opa_value *opa_number_int(long long v)
return &ret->hdr;
}

OPA_INTERNAL
opa_value *opa_number_float(double v)
{
opa_number_t *ret = (opa_number_t *)opa_malloc(sizeof(opa_number_t));
ret->hdr.type = OPA_NUMBER;
ret->repr = OPA_NUMBER_REPR_FLOAT;
ret->v.f = v;
return &ret->hdr;
}

OPA_INTERNAL
opa_value *opa_number_ref(const char *s, size_t len)
{
Expand Down Expand Up @@ -973,8 +953,6 @@ int opa_number_try_int(opa_number_t *n, long long *i)
{
switch (n->repr)
{
case OPA_NUMBER_REPR_FLOAT:
return -1;
case OPA_NUMBER_REPR_INT:
*i = n->v.i;
return 0;
Expand All @@ -990,8 +968,6 @@ double opa_number_as_float(opa_number_t *n)
{
switch (n->repr)
{
case OPA_NUMBER_REPR_FLOAT:
return n->v.f;
case OPA_NUMBER_REPR_INT:
return (double)n->v.i;
case OPA_NUMBER_REPR_REF:
Expand Down
4 changes: 1 addition & 3 deletions wasm/src/value.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ extern "C" {
#define OPA_BOOLEAN_INTERNED (9) // TODO(sr): make an "interned" bitmask?

#define OPA_NUMBER_REPR_INT (1)
#define OPA_NUMBER_REPR_FLOAT (2)
#define OPA_NUMBER_REPR_REF (3)
#define OPA_NUMBER_REPR_REF (2)

typedef struct opa_value opa_value;

Expand Down Expand Up @@ -49,7 +48,6 @@ typedef struct
unsigned char repr;
union {
long long i;
double f;
opa_number_ref_t ref;
} v;
} opa_number_t;
Expand Down
15 changes: 6 additions & 9 deletions wasm/tests/test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@
#include "test.h"
#include "types.h"

// NOTE(sr): we've removed the float number representation, so this helper
// is to make our tests less annoying:
#define opa_number_float(f) opa_number_ref(#f, sizeof(#f))

void reset_heap(void)
{
// This will leak memory!!
Expand Down Expand Up @@ -1253,15 +1257,8 @@ void test_opa_json_dump(void)
test("strings utf-8", opa_strcmp(opa_json_dump(opa_string_terminated("\xed\xba\xad")), "\"\xed\xba\xad\"") == 0);
test("numbers", opa_strcmp(opa_json_dump(opa_number_int(127)), "127") == 0);

// NOTE(tsandall): the string representation is lossy. We should store
// user-supplied floating-point values as strings so that round-trip
// operations are lossless. Computed values can be lossy for the time being.
test("numbers/float", opa_strcmp(opa_json_dump(opa_number_float(12345.678)), "12345.7") == 0);

// NOTE(tsandall): trailing zeros should be omitted but this appears to be an open issue: https://github.com/mpaland/printf/issues/55
test("numbers/float", opa_strcmp(opa_json_dump(opa_number_float(10.5)), "10.5000") == 0);

test("numbers/ref", opa_strcmp(opa_json_dump(opa_number_ref("127", 3)), "127") == 0);
test_str_eq("numbers/float", "12345.678", opa_json_dump(opa_number_float(12345.678)));
test_str_eq("numbers/float", "10.5", opa_json_dump(opa_number_float(10.5)));

opa_value *arr = opa_array();
test("arrays", opa_strcmp(opa_json_dump(arr), "[]") == 0);
Expand Down

0 comments on commit 8f91118

Please sign in to comment.