On 12/20/21 12:14 PM, Ævar Arnfjörð Bjarmason wrote:
On Mon, Dec 20 2021, Jeff Hostetler via GitGitGadget wrote:
From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
[...]
+ int nr_threads = 0;
+ int k;
+ pthread_t *pids = NULL;
+
+ if (argc != 3)
+ die("%s", usage_error);
+ if (get_i(&data.v1, argv[0]))
+ die("%s", usage_error);
+ if (get_i(&data.v2, argv[1]))
+ die("%s", usage_error);
+ if (get_i(&nr_threads, argv[2]))
+ die("%s", usage_error);
A partial nit on existing code, as this just extends the pattern, but
couldn't much of this get_i() etc. just be made redundant by simply
using the parse-options.c API here? I.e. OPTION_INTEGER and using named
arguments would do the validation or you.
I suppose. It just seemed like a little overkill setting things
up for such a simple and isolated test. And the cut-n-paste was
quick enough for my purposes.
+# Exercise the global counter in a loop and confirm that we get the
+# expected sum in an event record.
+#
+
+have_counter_event () {
+ thread=$1
+ name=$2
+ value=$3
+ file=$4
+
+ grep "\"event\":\"counter\".*\"thread\":\"${thread}\".*\"name\":\"${name}\".*\"value\":${value}" $file
+
+ return $?
+}
It looks like there's no helper, but this is the Nth thing I see where
wish our "test_region" helper were just a bit more generalized. I.e.:
test_trace2 --match=counter --match=thread=$thread --match=name=$name --match=value=$value <trace>
With test_region just being a wrapper for something like:
test_trace2 --match=region_enter --match=category=$category --match=label=$label <trace> &&
test_trace2 --match=region_leave --match=category=$category --match=label=$label <trace>
Yes, that would be nice. But I don't think we should
start that in the middle of this patch series. Perhaps
you could start a top-level message with a fleshed out
proposal and let everyone discuss it.
+static void tr2main_emit_summary_counters(uint64_t us_elapsed_absolute)
+{
+ struct tr2_tgt *tgt_j;
+ int j;
+ struct tr2ctr_block merged;
+
+ memset(&merged, 0, sizeof(merged));
nit: more memset v.s. "{ 0 }".
Yeah, but lldb wouldn't stop complaining until it was "= { { { 0 } } }"
Jeff
