diff mbox series

[RFC] pack-objects: write objects packed to trace2

Message ID 20190409214420.90898-1-jonathantanmy@google.com (mailing list archive)
State New, archived
Headers show
Series [RFC] pack-objects: write objects packed to trace2 | expand

Commit Message

Jonathan Tan April 9, 2019, 9:44 p.m. UTC
This is useful when investigating performance of pushes, and other times
when no progress information is written (because the pack is written to
stdout).

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
We're trying to improve push performance, and it would be nice to be
able to observe the number of objects sent over each push, both to
correlate it with time taken (which is already traced) and to notice
situations when significantly more objects are being sent than needed.

Sending this as an RFC because this patch works but is somewhat ad-hoc -
perhaps someone else has a more comprehensive solution.
---
 builtin/pack-objects.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Junio C Hamano April 10, 2019, 3:52 a.m. UTC | #1
Jonathan Tan <jonathantanmy@google.com> writes:

> This is useful when investigating performance of pushes, and other times
> when no progress information is written (because the pack is written to
> stdout).
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
> We're trying to improve push performance, and it would be nice to be
> able to observe the number of objects sent over each push, both to
> correlate it with time taken (which is already traced) and to notice
> situations when significantly more objects are being sent than needed.

Thanks---I find it a laudable goal, too.
Duy Nguyen April 10, 2019, 5:08 a.m. UTC | #2
On Wed, Apr 10, 2019 at 4:45 AM Jonathan Tan <jonathantanmy@google.com> wrote:
>
> This is useful when investigating performance of pushes, and other times
> when no progress information is written (because the pack is written to
> stdout).
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
> We're trying to improve push performance, and it would be nice to be
> able to observe the number of objects sent over each push, both to
> correlate it with time taken (which is already traced) and to notice
> situations when significantly more objects are being sent than needed.
>
> Sending this as an RFC because this patch works but is somewhat ad-hoc -
> perhaps someone else has a more comprehensive solution.
> ---
>  builtin/pack-objects.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index a154fc29f6..ac464d7d07 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -964,6 +964,7 @@ static void write_pack_file(void)
>         if (written != nr_result)
>                 die(_("wrote %"PRIu32" objects while expecting %"PRIu32),
>                     written, nr_result);
> +       trace2_printf("packed %d objects", nr_result);

The die() line right above uses %PRIu32 instead of %d

>  }
>
>  static int no_try_delta(const char *path)
> --
> 2.21.0.392.gf8f6787159e-goog
>
Jeff Hostetler April 10, 2019, 3:05 p.m. UTC | #3
On 4/9/2019 5:44 PM, Jonathan Tan wrote:
> This is useful when investigating performance of pushes, and other times
> when no progress information is written (because the pack is written to
> stdout).
> 
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
> We're trying to improve push performance, and it would be nice to be
> able to observe the number of objects sent over each push, both to
> correlate it with time taken (which is already traced) and to notice
> situations when significantly more objects are being sent than needed.
> 
> Sending this as an RFC because this patch works but is somewhat ad-hoc -
> perhaps someone else has a more comprehensive solution.
> ---
>   builtin/pack-objects.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index a154fc29f6..ac464d7d07 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -964,6 +964,7 @@ static void write_pack_file(void)
>   	if (written != nr_result)
>   		die(_("wrote %"PRIu32" objects while expecting %"PRIu32),
>   		    written, nr_result);
> +	trace2_printf("packed %d objects", nr_result);

For a simple field like this, you might want to use:

	trace2_data_intmax("pack-objects", the_repository,
			   "write_pack_file/wrote", nr_result);

This will give your message visibility in both the perf and event
targets.  The latter makes it easier for post-processing.

jeff
diff mbox series

Patch

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index a154fc29f6..ac464d7d07 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -964,6 +964,7 @@  static void write_pack_file(void)
 	if (written != nr_result)
 		die(_("wrote %"PRIu32" objects while expecting %"PRIu32),
 		    written, nr_result);
+	trace2_printf("packed %d objects", nr_result);
 }
 
 static int no_try_delta(const char *path)