Message ID | patch-v2-1.1-a1fc37de947-20220630T084607Z-avarab@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] trace2: don't include "fsync" events in all trace2 logs | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > As we're needing to indent the trace2_data_intmax() lines let's > introduce helper variables to ensure that our resulting lines (which > were already too) don't exceed the recommendations of the too -> too something > + elsif ($event eq 'data_json') { > + # NEEDSWORK: Ignore due to > + # compat/win32/trace2_win32_process_info.c, which should log a > + # "cmd_ancestry" event instead. > + } This does read better. > void trace_git_fsync_stats(void) > { > - trace2_data_intmax("fsync", the_repository, "fsync/writeout-only", count_fsync_writeout_only); > - trace2_data_intmax("fsync", the_repository, "fsync/hardware-flush", count_fsync_hardware_flush); > + const struct repository *r = the_repository; > + const intmax_t cfwo = count_fsync_writeout_only; > + const intmax_t cfhf = count_fsync_hardware_flush; > + > + if (cfwo) > + trace2_data_intmax("fsync", r, "fsync/writeout-only", cfwo); > + if (cfhf) > + trace2_data_intmax("fsync", r, "fsync/hardware-flush", cfhf); I would have wrapped the lines instead of introducing these extra variables; the key observation is that it would make a much easier pattern to follow for a future developer who needs to add the third kind of "fsync" on top of the existing wo and hf. But we can address that when somebody actually adds the third one, so let's take the patch as-is. Thanks.
diff --git a/t/t0212/parse_events.perl b/t/t0212/parse_events.perl index b6408560c0c..30a9f51e9f1 100644 --- a/t/t0212/parse_events.perl +++ b/t/t0212/parse_events.perl @@ -216,12 +216,19 @@ elsif ($event eq 'data') { my $cat = $line->{'category'}; - if ($cat eq 'test_category') { - - my $key = $line->{'key'}; - my $value = $line->{'value'}; - $processes->{$sid}->{'data'}->{$cat}->{$key} = $value; - } + my $key = $line->{'key'}; + my $value = $line->{'value'}; + $processes->{$sid}->{'data'}->{$cat}->{$key} = $value; + } + + elsif ($event eq 'data_json') { + # NEEDSWORK: Ignore due to + # compat/win32/trace2_win32_process_info.c, which should log a + # "cmd_ancestry" event instead. + } + + else { + push @{$processes->{$sid}->{$event}} => $line->{value}; } # This trace2 target does not emit 'printf' events. diff --git a/wrapper.c b/wrapper.c index 1c3c970080b..67727f95411 100644 --- a/wrapper.c +++ b/wrapper.c @@ -618,8 +618,14 @@ int git_fsync(int fd, enum fsync_action action) void trace_git_fsync_stats(void) { - trace2_data_intmax("fsync", the_repository, "fsync/writeout-only", count_fsync_writeout_only); - trace2_data_intmax("fsync", the_repository, "fsync/hardware-flush", count_fsync_hardware_flush); + const struct repository *r = the_repository; + const intmax_t cfwo = count_fsync_writeout_only; + const intmax_t cfhf = count_fsync_hardware_flush; + + if (cfwo) + trace2_data_intmax("fsync", r, "fsync/writeout-only", cfwo); + if (cfhf) + trace2_data_intmax("fsync", r, "fsync/hardware-flush", cfhf); } static int warn_if_unremovable(const char *op, const char *file, int rc)