This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Tue, Apr 08, 2014 at 10:28:10AM +0100, Will Newton wrote: > Add a small library to print JSON values and use it to improve the > readability of the benchmark output and the readability of the > benchmark code. Sorry about the delay, but here's the review. The patch looks good to me barring a few nits that you can fix before committing. Thanks, Siddhesh > > ChangeLog: > > 2014-04-08 Will Newton <will.newton@linaro.org> > > * benchtests/Makefile (extra-objs): Add json-lib.o. > (bench-func): Tidy up JSON output. > * benchtests/bench-skeleton.c: Include json-lib.h. > (main): Use JSON library functions to do output of > benchmark results. > * benchtests/bench-timing-type.c (main): Output the > timing type simply, leaving formatting to the user. > * benchtests/json-lib.c: New file. > * benchtests/json-lib.h: Likewise. > --- > benchtests/Makefile | 13 ++-- > benchtests/bench-skeleton.c | 39 ++++++------ > benchtests/bench-timing-type.c | 2 +- > benchtests/json-lib.c | 132 +++++++++++++++++++++++++++++++++++++++++ > benchtests/json-lib.h | 42 +++++++++++++ > 5 files changed, 205 insertions(+), 23 deletions(-) > create mode 100644 benchtests/json-lib.c > create mode 100644 benchtests/json-lib.h > > diff --git a/benchtests/Makefile b/benchtests/Makefile > index ca635cf..a788082 100644 > --- a/benchtests/Makefile > +++ b/benchtests/Makefile > @@ -100,6 +100,8 @@ cpp-srcs-left := $(binaries-benchset:=.c) $(binaries-bench:=.c) > lib := nonlib > include $(patsubst %,$(..)cppflags-iterator.mk,$(cpp-srcs-left)) > > +extra-objs += json-lib.o > + > bench-deps := bench-skeleton.c bench-timing.h Makefile > > run-bench = $(test-wrapper-env) \ > @@ -126,9 +128,9 @@ bench-set: $(binaries-benchset) > # so one could even execute them individually and process it using any JSON > # capable language or tool. > bench-func: $(binaries-bench) > - { echo "{"; \ > - $(timing-type); \ > - echo " ,\"functions\": {"; \ > + { timing_type=$$($(timing-type)); \ > + echo "{\"timing_type\": \"$${timing_type}\","; \ > + echo " \"functions\": {"; \ > for run in $^; do \ > if ! [ "x$${run}" = "x$<" ]; then \ > echo ","; \ > @@ -136,14 +138,15 @@ bench-func: $(binaries-bench) > echo "Running $${run}" >&2; \ > $(run-bench) $(DETAILED_OPT); \ > done; \ > - echo " }"; \ > + echo; \ > + echo " }"; \ > echo "}"; } > $(objpfx)bench.out-tmp; \ > if [ -f $(objpfx)bench.out ]; then \ > mv -f $(objpfx)bench.out $(objpfx)bench.out.old; \ > fi; \ > mv -f $(objpfx)bench.out-tmp $(objpfx)bench.out > > -$(timing-type) $(binaries-bench) $(binaries-benchset): %: %.o \ > +$(timing-type) $(binaries-bench) $(binaries-benchset): %: %.o $(objpfx)json-lib.o \ > $(sort $(filter $(common-objpfx)lib%,$(link-libc))) \ > $(addprefix $(csu-objpfx),start.o) $(+preinit) $(+postinit) > $(+link) > diff --git a/benchtests/bench-skeleton.c b/benchtests/bench-skeleton.c > index 0c7d744..ee37173 100644 > --- a/benchtests/bench-skeleton.c > +++ b/benchtests/bench-skeleton.c > @@ -23,6 +23,7 @@ > #include <time.h> > #include <inttypes.h> > #include "bench-timing.h" > +#include "json-lib.h" > > volatile unsigned int dontoptimize = 0; > > @@ -50,6 +51,7 @@ main (int argc, char **argv) > struct timespec runtime; > timing_t start, end; > bool detailed = false; > + json_ctx_t json_ctx; > > if (argc == 2 && !strcmp (argv[1], "-d")) > detailed = true; > @@ -64,13 +66,13 @@ main (int argc, char **argv) > > iters = 1000 * res; > > + json_init (&json_ctx, 2, stdout); > + > /* Begin function. */ > - printf ("\"%s\": {\n", FUNCNAME); > + json_object_begin (&json_ctx, FUNCNAME); > > for (int v = 0; v < NUM_VARIANTS; v++) > { > - if (v) > - putc (',', stdout); > /* Run for approximately DURATION seconds. */ > clock_gettime (CLOCK_MONOTONIC_RAW, &runtime); > runtime.tv_sec += DURATION; > @@ -119,28 +121,31 @@ main (int argc, char **argv) > d_total_s = total; > d_iters = iters; > > - printf ("\"%s\": {\n", VARIANT (v)); > - printf ("\"duration\": %g, \"iterations\": %g, " > - "\"max\": %g, \"min\": %g, \"mean\": %g\n", > - d_total_s, d_total_i, max / d_iters, min / d_iters, > - d_total_s / d_total_i); > + /* Begin variant. */ > + json_object_begin (&json_ctx, VARIANT (v)); > + > + json_attr_double (&json_ctx, "duration", d_total_s); > + json_attr_double (&json_ctx, "iterations", d_total_i); > + json_attr_double (&json_ctx, "max", max / d_iters); > + json_attr_double (&json_ctx, "min", min / d_iters); > + json_attr_double (&json_ctx, "mean", d_total_s / d_total_i); > > if (detailed) > { > - printf (",\n\"timings\": ["); > + json_array_begin (&json_ctx, "timings"); > + > for (int i = 0; i < NUM_SAMPLES (v); i++) > - { > - if (i > 0) > - putc (',', stdout); > - printf ("%g", RESULT (v, i)); > - } > - puts ("]"); > + json_element_double (&json_ctx, RESULT (v, i)); > + > + json_array_end (&json_ctx); > } > - puts ("}"); > + > + /* End variant. */ > + json_object_end (&json_ctx); > } > > /* End function. */ > - puts ("}"); > + json_object_end (&json_ctx); > > return 0; > } > diff --git a/benchtests/bench-timing-type.c b/benchtests/bench-timing-type.c > index 903a61f..64219ff 100644 > --- a/benchtests/bench-timing-type.c > +++ b/benchtests/bench-timing-type.c > @@ -22,6 +22,6 @@ > int > main (int argc, char **argv) > { > - printf ("\"timing-type\": \"%s\"\n", TIMING_TYPE); > + puts (TIMING_TYPE); > return 0; > } > diff --git a/benchtests/json-lib.c b/benchtests/json-lib.c > new file mode 100644 > index 0000000..87f21ae > --- /dev/null > +++ b/benchtests/json-lib.c > @@ -0,0 +1,132 @@ > +/* Simple library for printing JSON data. > + Copyright (C) 2014 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <string.h> > + > +#include "json-lib.h" > + > +void > +json_init (json_ctx_t *ctx, unsigned int indent_level, FILE *fp) > +{ > + ctx->indent_level = indent_level; > + ctx->fp = fp; > + ctx->first_element = true; > +} > + > +static void > +do_indent (json_ctx_t *ctx) > +{ > + char indent_buf[ctx->indent_level + 1]; > + > + memset (indent_buf, ' ', ctx->indent_level + 1); > + indent_buf[ctx->indent_level] = '\0'; > + > + fputs (indent_buf, ctx->fp); > +} > + > +void > +json_object_begin (json_ctx_t *ctx, const char *name) > +{ > + if (!ctx->first_element) > + fprintf (ctx->fp, ",\n"); > + > + do_indent (ctx); > + > + fprintf (ctx->fp, "\"%s\": {\n", name); > + > + ctx->indent_level++; > + ctx->first_element = true; > +} > + > +void > +json_object_end (json_ctx_t *ctx) > +{ > + ctx->indent_level--; > + ctx->first_element = false; > + > + fputs ("\n", ctx->fp); > + > + do_indent (ctx); > + > + fputs ("}", ctx->fp); > +} > + > +void > +json_array_begin (json_ctx_t *ctx, const char *name) > +{ > + if (!ctx->first_element) > + fprintf (ctx->fp, ",\n"); > + > + do_indent (ctx); > + > + fprintf (ctx->fp, "\"%s\": [", name); > + > + ctx->indent_level++; > + ctx->first_element = true; > +} > + > +void > +json_element_double (json_ctx_t *ctx, double d) > +{ > + if (!ctx->first_element) > + fprintf (ctx->fp, ", %g", d); > + else > + { > + fprintf (ctx->fp, "%g", d); > + ctx->first_element = false; > + } > +} > + > +void > +json_array_end (json_ctx_t *ctx) > +{ > + ctx->indent_level--; > + ctx->first_element = false; > + > + fputs ("]", ctx->fp); > +} > + > +void > +json_attr_string (json_ctx_t *ctx, const char *name, const char *s) > +{ > + if (!ctx->first_element) > + { > + fprintf (ctx->fp, ",\n"); > + } Redundant braces. > + > + ctx->first_element = false; It would be nicer to put this in an else block to get rid of a redundant assignment. I guess the compiler must be doing that anyway though. I don't have a very strong opinion on this though, since it is not exactly performance sensitive. > + > + do_indent (ctx); > + > + fprintf (ctx->fp, "\"%s\": \"%s\"", name, s); > +} > + > +void > +json_attr_double (json_ctx_t *ctx, const char *name, double d) > +{ > + if (!ctx->first_element) > + { > + fprintf (ctx->fp, ",\n"); > + } > + Redundant braces. > + ctx->first_element = false; > + Maybe put in else block as suggested above. > + do_indent (ctx); > + > + fprintf (ctx->fp, "\"%s\": %g", name, d); > +} > diff --git a/benchtests/json-lib.h b/benchtests/json-lib.h > new file mode 100644 > index 0000000..3cbbab3 > --- /dev/null > +++ b/benchtests/json-lib.h > @@ -0,0 +1,42 @@ > +/* Simple library for printing JSON data. > + Copyright (C) 2014 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#ifndef __JSON_LIB_H__ > +#define __JSON_LIB_H__ > + > +#include <stdbool.h> > +#include <stdio.h> > + > +struct json_ctx { The brace should be on the next line. > + FILE *fp; > + unsigned int indent_level; > + bool first_element; > +}; > + > +typedef struct json_ctx json_ctx_t; > + > +void json_init (json_ctx_t *ctx, unsigned int indent_level, FILE *fp); > +void json_object_begin (json_ctx_t *ctx, const char *name); > +void json_object_end (json_ctx_t *ctx); > +void json_array_begin (json_ctx_t *ctx, const char *name); > +void json_element_double (json_ctx_t *ctx, double d); > +void json_array_end (json_ctx_t *ctx); > +void json_attr_string (json_ctx_t *ctx, const char *name, const char *s); > +void json_attr_double (json_ctx_t *ctx, const char *name, double d); > + > +#endif > -- > 1.8.1.4 >
Attachment:
pgpEecrRjzSI5.pgp
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |