rib / gputop

A GPU profiling tool
160 stars 37 forks source link

immutable metric set data and stream state should be decoupled #127

Closed rib closed 8 years ago

rib commented 8 years ago

Currently we have struct gputop_perf_query (which mostly represents the meta data of the XML metric sets in the form of C structures, generated via oa-gen.xml) also coupled with a uint64_t accumulator[] array which is runtime state used in the processing of the metrics, aggregating counter deltas over a configurable period of time before evaluating the normalization equations.

One limitation of coupling these is that we can't have multiple accumulators per stream of metrics, which would be useful when we want to be able to visualize the same metrics in multiple ways at the same time, depending of accumulating for different lengths of time. For example in the web ui while plotting a trace of some metrics on a graph we want very short accumulation periods so we can plot lots of points over time for a detailed graph while we also want to show the bar graphs for counters at the same time which should update less frequently, maybe just once per second, so the values remain readable to the user.

We should aim to introduce a separate struct gputop_perf_aggregator that is associated with a stream of metrics instead of being coupled with the metric set descriptions.

Assuming we rename struct gputop_perf_query to struct gputop_metric_set and struct gputop_worker_query to struct gputop_remote_stream as in #126 then we could introduce a struct gputop_perf_aggregator that itself works with the data from a struct gputop_remote_stream.

The aggregator would track the uint64_t array of deltas and a period over which counter deltas are accumulated.

When the raw i915 perf samples are processed the same raw counter reports should be processed separately for each accumulator.

sergioamr commented 8 years ago

I am working on this with a bit of a different approach: https://github.com/sergioamr/gputop/blob/sergio/wip/oa-pid-test/gputop/gputop-accumulator.c

From: Robert Bragg [mailto:notifications@github.com] Sent: Wednesday, March 2, 2016 8:23 PM To: rib/gputop gputop@noreply.github.com Subject: [gputop] immutable metric set data and stream state should be decoupled (#127)

Currently we have struct gputop_perf_query (which mostly represents the meta data of the XML metric sets in the form of C structures, generated via oa-gen.xml) also coupled with a uint64_t accumulator[] array which is runtime state used in the processing of the metrics, aggregating counter deltas over a configurable period of time before evaluating the normalization equations.

One limitation of coupling these is that we can't have multiple accumulators per stream of metrics, which would be useful when we want to be able to visualize the same metrics in multiple ways at the same time, depending of accumulating for different lengths of time. For example in the web ui while plotting a trace of some metrics on a graph we want very short accumulation periods so we can plot lots of points over time for a detailed graph while we also want to show the bar graphs for counters at the same time which should update less frequently, maybe just once per second, so the values remain readable to the user.

We should aim to introduce a separate struct gputop_perf_aggregator that is associated with a stream of metrics instead of being coupled with the metric set descriptions.

Assuming we rename struct gputop_perf_query to struct gputop_metric_set and struct gputop_worker_query to struct gputop_remote_stream as in #126https://github.com/rib/gputop/issues/126 then we could introduce a struct gputop_perf_aggregator that itself works with the data from a struct gputop_remote_stream.

The aggregator would track the uint64_t array of deltas and a period over which counter deltas are accumulated.

When the raw i915 perf samples are processed the same raw counter reports should be processed separately for each accumulator.

Reply to this email directly or view it on GitHubhttps://github.com/rib/gputop/issues/127.

Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

rib commented 8 years ago

At least as far as adding a struct gputop_oa_accumulator that's separate from gputop_perf_query/metric_set this was resolved in 6979e0a63ad3e72cc2c50f7c40cec4ee22326103

Further features based on this such as the pid filtering being investigated by @sergioamr or having a separate accumulator for the slower bar graph updates can be tracked as separate issues.