Skip to content

Commit

Permalink
optimize cpu_profile_transformer to not create intermediate lists (fl…
Browse files Browse the repository at this point in the history
…utter#8839)

Uses an iterator over the stack frame values in CpuProfileTransformer, instead of allocating new lists for they keys and values in the map, which are then read by index. I also removed the late/nullable fields in favor of local variables, since the state was never used outside of a single method call.

As far as I could tell the keys were never actually used except to look up values in the map, so I dropped that entirely. There was an extra lookup in the map and local variable which I am pretty certain wasn't needed.

This isn't directly related to any specific issue, I just noticed it as I was perusing through some of the code for the cpu profiler and figured I would send something out, to dip my toes in here :).

No tests were added, the behavior should be unchanged so hopefully existing tests should cover it.

I did also add a TODO because I noticed a piece of logic that was O(N^2) worst case here but I don't know enough about how large the N is expected to be or whether its a common path, and it would be more involved to fix.
  • Loading branch information
jakemac53 authored Feb 3, 2025
1 parent b38abb8 commit b6b0e43
Showing 1 changed file with 16 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,6 @@ class CpuProfileTransformer {
/// Number of stack frames we will process in each batch.
static const _defaultBatchSize = 100;

late int _stackFramesCount;

List<String?>? _stackFrameKeys;

List<CpuStackFrame>? _stackFrameValues;

int _stackFramesProcessed = 0;

String? _activeProcessId;
Expand All @@ -34,21 +28,25 @@ class CpuProfileTransformer {
reset();

_activeProcessId = processId;
_stackFramesCount = cpuProfileData.stackFrames.length;
_stackFrameKeys = cpuProfileData.stackFrames.keys.toList();
_stackFrameValues = cpuProfileData.stackFrames.values.toList();
final stackFramesCount = cpuProfileData.stackFrames.length;
final stackFrameValues = cpuProfileData.stackFrames.values.iterator;

// At minimum, process the data in 4 batches to smooth the appearance of
// the progress indicator.
final quarterBatchSize = (_stackFramesCount / 4).round();
final quarterBatchSize = (stackFramesCount / 4).round();
final batchSize = math.min(
_defaultBatchSize,
quarterBatchSize == 0 ? 1 : quarterBatchSize,
);

// Use batch processing to maintain a responsive UI.
while (_stackFramesProcessed < _stackFramesCount) {
_processBatch(batchSize, cpuProfileData, processId: processId);
while (_stackFramesProcessed < stackFramesCount) {
_processBatch(
batchSize,
cpuProfileData,
processId: processId,
stackFrameValues: stackFrameValues,
);

// Await a small delay to give the UI thread a chance to update the
// progress indicator. Use a longer delay than the default (0) so that the
Expand All @@ -74,6 +72,8 @@ class CpuProfileTransformer {
nodeIndicesToRemove.add(i);
}
}
// TODO(jakemac53): This is O(N^2), we should have a function to remove
// multiple children at once.
nodeIndicesToRemove.forEach(
cpuProfileData.cpuProfileRoot.removeChildAtIndex,
);
Expand All @@ -98,19 +98,14 @@ class CpuProfileTransformer {
int batchSize,
CpuProfileData cpuProfileData, {
required String processId,
required Iterator<CpuStackFrame> stackFrameValues,
}) {
final batchEnd = math.min(
_stackFramesProcessed + batchSize,
_stackFramesCount,
);
for (int i = _stackFramesProcessed; i < batchEnd; i++) {
for (int i = 0; i < batchSize && stackFrameValues.moveNext(); i++) {
if (processId != _activeProcessId) {
throw ProcessCancelledException();
}
final key = _stackFrameKeys![i];
final value = _stackFrameValues![i];
final stackFrame = cpuProfileData.stackFrames[key]!;
final parent = cpuProfileData.stackFrames[value.parentId];
final stackFrame = stackFrameValues.current;
final parent = cpuProfileData.stackFrames[stackFrame.parentId];
_processStackFrame(stackFrame, parent, cpuProfileData);
_stackFramesProcessed++;
}
Expand Down Expand Up @@ -150,8 +145,6 @@ class CpuProfileTransformer {
void reset() {
_activeProcessId = null;
_stackFramesProcessed = 0;
_stackFrameKeys = null;
_stackFrameValues = null;
}
}

Expand Down

0 comments on commit b6b0e43

Please sign in to comment.