On Wed, Oct 11, 2023 at 04:04:02PM +0000, Robert Coup via GitGitGadget wrote: > Improve this by emitting a Trace2 JSON event from upload-pack with > summary information on the contents of a fetch request. > > * haves, wants, and want-ref counts can help determine (broadly) between > fetches and clones, and the use of single-branch, etc. > * shallow clone depth, tip counts, and deepening options. > * any partial clone filter type. I think this is a reasonable thing to have. I see Taylor left a more detailed review, but I did notice one thing... > +static void trace2_fetch_info(struct upload_pack_data *data) > +{ > + struct json_writer jw = JSON_WRITER_INIT; > + > + jw_object_begin(&jw, 0); > + { > + jw_object_intmax(&jw, "haves", data->haves.nr); > + jw_object_intmax(&jw, "wants", data->want_obj.nr); > + jw_object_intmax(&jw, "want-refs", data->wanted_refs.nr); > + jw_object_intmax(&jw, "depth", data->depth); > + jw_object_intmax(&jw, "shallows", data->shallows.nr); > + jw_object_bool(&jw, "deepen-since", data->deepen_since); > + jw_object_intmax(&jw, "deepen-not", data->deepen_not.nr); > + jw_object_bool(&jw, "deepen-relative", data->deepen_relative); > + if (data->filter_options.choice) > + jw_object_string(&jw, "filter", list_object_filter_config_name(data->filter_options.choice)); > + else > + jw_object_null(&jw, "filter"); > + } > + jw_end(&jw); > + > + trace2_data_json("upload-pack", the_repository, "fetch-info", &jw); > + > + jw_release(&jw); > +} Generating the json output isn't entirely trivial (and certainly involves allocations), but we throw it away unused if tracing isn't enabled. Maybe we'd want something like: if (!trace2_is_enabled()) return; at the top of the function? It looks like other callers of jw_object_begin() have a similar issue, and this is probably premature optimization to some degree. It just feels like it should be easy for tracing to be zero-cost (beyond a single conditional) when it's disabled. -Peff