For processing optimization samples for visualizing JIT tiers over time.

Changed so now data points with no opt sites will return an object {x: time, ys:[0,0,0]} https://treeherder.mozilla.org/#/jobs?repo=try&revision=2dedede48bc1

Comment on attachment 8623832 [details] [diff] [review] 1174889-opttiers.patch Review of attachment 8623832 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/performance/modules/logic/jit.js @@ +289,5 @@ > + let stringTable = frameNode._stringTable; > + let output = []; > + let implEnum; > + > + let tierDataIterator = 0; Nit: this is an index, not an Iterator. @@ +304,5 @@ > + for (let i = 0; i <= sampleTimes.length; i++) { > + let sampleTime = sampleTimes[i]; > + > + // If this sample is in the next bucket, or we're done > + // checking sampleTimes and on the last loop, finalize previous bucket last iteration, not last loop. @@ +306,5 @@ > + > + // If this sample is in the next bucket, or we're done > + // checking sampleTimes and on the last loop, finalize previous bucket > + if (sampleTime >= (currentBucketStartTime + bucketSize) || > + i >= sampleTimes.length) { This should also make sure there's actual samples in the current bucket. See below. @@ +315,5 @@ > + > + // Map the opt site counts as a normalized percentage (0-1) > + // of its count in context of total samples this bucket > + for (let j = 0; j < IMPLEMENTATION_NAMES.length; j++) { > + dataPoint.ys[j] = (bucket[j] || 0) / samplesInCurrentBucket; This has the potential to divide by 0 if the bucket size is smaller than the profiling frequency. ::: browser/devtools/performance/modules/logic/tree-model.js @@ +229,5 @@ > if (isLeaf) { > frameNode.youngestFrameSamples++; > + if (inflatedFrame.optimizations) { > + frameNode._addOptimizations(inflatedFrame.optimizations, inflatedFrame.implementation, > + sampleTime, stringTable); This begs for an { object param } @@ +239,5 @@ > prevCalls = frameNode.calls; > isLeaf = mutableFrameKeyOptions.isLeaf = false; > } > > this.samples++; We don't need the `samples` property anymore right? Could just check sampleTimes.length.

Comment on attachment 8623832 [details] [diff] [review] 1174889-opttiers.patch Review of attachment 8623832 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/performance/modules/logic/jit.js @@ +315,5 @@ > + > + // Map the opt site counts as a normalized percentage (0-1) > + // of its count in context of total samples this bucket > + for (let j = 0; j < IMPLEMENTATION_NAMES.length; j++) { > + dataPoint.ys[j] = (bucket[j] || 0) / samplesInCurrentBucket; Opting to just do `samplesInCurrentBucket || 1` here to prevent dividing by 0. If this aborted when checking if there were any samples, then we wouldn't have an empty bucket here, which we should to be consistent with other empty buckets (where samples exist, but no opt data) ::: browser/devtools/performance/modules/logic/tree-model.js @@ +229,5 @@ > if (isLeaf) { > frameNode.youngestFrameSamples++; > + if (inflatedFrame.optimizations) { > + frameNode._addOptimizations(inflatedFrame.optimizations, inflatedFrame.implementation, > + sampleTime, stringTable); I agree, but nothing in this, and frame-utils does that, I think for performance reasons. I don't know @@ +239,5 @@ > prevCalls = frameNode.calls; > isLeaf = mutableFrameKeyOptions.isLeaf = false; > } > > this.samples++; Once bug 1175705 is landed, I want to make this.sampleTimes only accumulate if optimizations are on, so we don't have to store a potentially large extra array for no reason