Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/components/shared/thread/ActivityGraphFills.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export function computeActivityGraphFills(
mutableFills
);

const upperGraphEdge = activityGraphFills.run();
const { averageCPUPerPixel, upperGraphEdge } = activityGraphFills.run();
// We're done mutating the fills' Float32Array buffers.
const fills = mutableFills;

Expand All @@ -123,6 +123,7 @@ export function computeActivityGraphFills(
fillsQuerier: new ActivityFillGraphQuerier(
renderedComponentSettings,
fills,
averageCPUPerPixel,
upperGraphEdge
),
};
Expand Down Expand Up @@ -152,11 +153,18 @@ export class ActivityGraphFillComputer {
* Run the computation to compute a list of the fills that need to be drawn for the
* ThreadActivityGraph.
*/
run(): Float32Array {
run(): {|
+averageCPUPerPixel: Float32Array,
+upperGraphEdge: Float32Array,
|} {
// First go through each sample, and set the buffers that contain the percentage
// that a category contributes to a given place in the X axis of the chart.
this._accumulateSampleCategories();

// First get the average CPU in each pixel, and then accumulate the upper edge
// of the graph after applying the blur.
const averageCPUPerPixel = this._accumulateUpperEdge().slice();

// Smooth the graphs by applying a 1D gaussian blur to the per-pixel
// contribution of each fill.
for (const fill of this.mutableFills) {
Expand All @@ -165,7 +173,7 @@ export class ActivityGraphFillComputer {

const upperGraphEdge = this._accumulateUpperEdge();

return upperGraphEdge;
return { averageCPUPerPixel, upperGraphEdge };
}

/**
Expand Down Expand Up @@ -383,15 +391,18 @@ export class ActivityGraphFillComputer {
export class ActivityFillGraphQuerier {
renderedComponentSettings: RenderedComponentSettings;
fills: CategoryFill[];
averageCPUPerPixel: Float32Array;
upperGraphEdge: Float32Array;

constructor(
renderedComponentSettings: RenderedComponentSettings,
fills: CategoryFill[],
averageCPUPerPixel: Float32Array,
upperGraphEdge: Float32Array
) {
this.renderedComponentSettings = renderedComponentSettings;
this.fills = fills;
this.averageCPUPerPixel = averageCPUPerPixel;
this.upperGraphEdge = upperGraphEdge;
}

Expand Down Expand Up @@ -485,9 +496,7 @@ export class ActivityFillGraphQuerier {
return null;
}

// This is the height of the graph and it directly corresponds to the average
// CPU usage number.
const cpuRatio = this.upperGraphEdge[deviceX];
const cpuRatio = this.averageCPUPerPixel[deviceX];

// Get the time range of the contributed samples to the average CPU usage value.
let timeRange = 0;
Expand Down
46 changes: 38 additions & 8 deletions src/test/components/SampleTooltipContents.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,38 @@ const GRAPH_WIDTH = Math.floor(PIXELS_PER_SAMPLE * SAMPLE_COUNT);
const GRAPH_HEIGHT = 10;
function getSamplesPixelPosition(
sampleIndex: IndexIntoSamplesTable,
samplePosition: 'before' | 'after' = 'before'
samplePosition: 'before' | 'after' | 'on-top' = 'before'
): CssPixels {
const quarterSample = PIXELS_PER_SAMPLE / 4;
// Compute the pixel position of either first or the second part of the sample.
// "sampleIndex * PIXELS_PER_SAMPLE" corresponds to the center of the sample,
// we need to +/- quarter sample to find these places in the sample:
//
// before after
// 50% CPU 100% CPU
// v v
// on-top
// 100% CPU
// |
// before | after
// 50% CPU | 100% CPU
// v v v
// +--------------------+--------------------+--------------------+

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: please update the comment here :-)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, thanks!

// | | //////////| |
// +--------------------+////////////////////+--------------------+
const beforeOrAfter =
samplePosition === 'before' ? -quarterSample : +quarterSample;
return sampleIndex * PIXELS_PER_SAMPLE + beforeOrAfter;
let sampleOffset;
switch (samplePosition) {
case 'before':
sampleOffset = -quarterSample;
break;
case 'after':
sampleOffset = +quarterSample;
break;
case 'on-top':
sampleOffset = 0;
break;

default:
break;
}
return sampleIndex * PIXELS_PER_SAMPLE + sampleOffset;
}

describe('SampleTooltipContents', function () {
Expand Down Expand Up @@ -85,7 +101,7 @@ describe('SampleTooltipContents', function () {
function setup(
profile: Profile,
hoveredSampleIndex: number,
hoveredSamplePosition: 'before' | 'after' = 'before'
hoveredSamplePosition: 'before' | 'after' | 'on-top' = 'before'
) {
const store = storeWithProfile(profile);
const threadsKey = 0;
Expand Down Expand Up @@ -312,4 +328,18 @@ describe('SampleTooltipContents', function () {
const cpuUsage = ensureExists(screen.getByText(/CPU/).nextElementSibling);
expect(cpuUsage).toHaveTextContent('45% (average over 1.0ms)');
});

it('renders the CPU usage properly in the areas where blur is applied', () => {
const profile = getProfileWithCPU([null, 400, 580, 1000], 'µs');

// Let's check the second threadCPUDelta value. This should show the
// We are hovering on top of the sample which should use 580µs. Since the
// max value is 1000µs, the CPU usage should be 58%.
const hoveredSampleIndex = 1;
// Hovering on top of the sample.
setup(profile, hoveredSampleIndex, 'on-top');

const cpuUsage = ensureExists(screen.getByText(/CPU/).nextElementSibling);
expect(cpuUsage).toHaveTextContent('58% (average over 1.0ms)');
});
});