r? shu for the tree model stuff; vp for everything else. Nick, f?ing you for checking out mostly the tests to see if those are the expected values that we should get in a normal and inverted tree. https://treeherder.mozilla.org/#/jobs?repo=try&revision=96606f9f6295

Comment on attachment 8650541 [details] [diff] [review] 1194458-alloc-bytesize.patch Review of attachment 8650541 [details] [diff] [review]: ----------------------------------------------------------------- Nice integration test coverage. ::: browser/devtools/performance/test/unit/test_tree-model-allocations-02.js @@ +37,5 @@ > + [100, 10, 1, 33, 100, 10, 1, 33, "x (A:1:2)"] > + ].forEach(compareFrameInfo(thread)); > +}); > + > +function compareFrameInfo (root, parent) { I feel like this could be pulled out of the test_tree-model-allocations-0{1,2}.js files and shared in a head file. Then you could also take the opportunity to document its use and params for future test readers.

Comment on attachment 8650541 [details] [diff] [review] 1194458-alloc-bytesize.patch Review of attachment 8650541 [details] [diff] [review]: ----------------------------------------------------------------- r=me for frame-utils and tree-model with comments applied. ::: browser/devtools/performance/modules/logic/frame-utils.js @@ +511,4 @@ > data.selfCount = node.youngestFrameSamples; > data.totalCount = node.samples; > + data.selfCountPercentage = node.youngestFrameSamples / totalSamples * 100; > + data.totalCountPercentage = node.samples / totalSamples * 100; So |data| has different field names for the total sample % depending on if allocation data is displayed or not? Could you unify them? ::: browser/devtools/performance/modules/logic/tree-model.js @@ +36,5 @@ > this.calls = []; > this.duration = options.endTime - options.startTime; > this.nodeType = "Thread"; > + // Total bytesize of all allocations if enabled > + this.totalBytes = 0; I suggest renaming this to byteSize for consistency. Also assign youngestFrameByteSize = 0 here. @@ +109,5 @@ > } > > const SAMPLE_STACK_SLOT = samples.schema.stack; > const SAMPLE_TIME_SLOT = samples.schema.time; > + const SAMPLE_BYTESIZE_SLOT = samples.schema.size; I suggest setting this conditionally, depending on if samples.schema has a 'size' property. @@ +139,5 @@ > > for (let i = 0; i < samplesData.length; i++) { > let sample = samplesData[i]; > let sampleTime = sample[SAMPLE_TIME_SLOT]; > + let byteSize = sample[SAMPLE_BYTESIZE_SLOT]; I suggest hoisting byteSize out of the loop and initializing it to 0. Then do something like the following here: if (SAMPLE_BYTESIZE_SLOT !== undefined) { byteSize = sample[SAMPLE_BYTESIZE_SLOT]; } and making all the additions below unconditional. @@ +291,5 @@ > > let node = entry.node; > let calls = node.calls; > let callSamples = 0; > + let totalSize = 0; nit: 'total' is kind of misleading here. Maybe 'callByteSize'?

Updated, waiting for tree to open https://treeherder.mozilla.org/#/jobs?repo=try&revision=44e466f30477

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7a4e612752b