Re: [PATCH] upload-pack: add tracing for fetches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux