Deploy Jan 30, 2025#5348
Merged
Merged
Conversation
I think I was trying to avoid line breaks at the time, but it's just a weird name. Let's rename it to something that makes sense.
Three more preparations for #5287, no changes in behavior.
Co-authored-by: Jim Spentzos <jimspentzos2000@gmail.com> (el)
…ndex. (#5329) It was only using it for the stringTable, and only looking up string indexes found on the unfiltered thread, so it might as well use the unfiltered thread's stringTable. It's getting the filtered samples through a separate argument.
… summary strategy"-aware samples. Not a huge fan of the name "CtssSamples" but at least it's now different. This patch makes it obvious we have a few bugs.
…iltered call tree summary strategy samples when using an allocation strategy. Fixes #5331. Let's say we allocate 100 objects, and then free the first 50 of them. If you look at the entire thread, the `retained-native-allocations` view will show the allocation samples for the second 50 objects (the ones that weren't freed). If you look at just the part of the thread where you allocated, you will see allocation samples for all 100 objects (because none were freed in that part of the thread). How do you get the stack category for each of the 100 allocation samples, especially if you've applied transforms or a JS-only filter? You need to look at the stack of the raw nativeAllocations table. You cannot look at the `retained-native-allocations` of the entire thread, because that table doesn't have the stacks for the first 50 objects - it removes those samples because it knows that those objects have been deallocated. This commit makes it so that the unfilteredCtssSamples are just the raw allocation tables, when an allocation-related call tree summary strategy is selected. And it makes it so that `extractSamplesLikeTable` always returns a table that has compatible indexes to the `extractUnfilteredCtssSamples`, just with some samples having a null stack. Now the unfilteredCtssSamples can be used to look up the stack, and therefore the original category of a sample, based on the sample index in the filtered (or preview-filtered) CTSS samples table. We should not create a call tree just based on the unfilteredCtssSamples because these samples have not been filtered according to the strategy.
Here is everything you need to know about this upgrade. Please take a good look at what changed and the test results before merging this pull request. ### What changed? #### ✳️ stylelint-config-standard (36.0.1 → 37.0.0) · [Repo](https://github.com/stylelint/stylelint-config-standard) · [Changelog](https://github.com/stylelint/stylelint-config-standard/blob/main/CHANGELOG.md)
Co-authored-by: Julien Wajsberg <felash@gmail.com>
…iltered call tree summary strategy samples when using an allocation strategy (#5332) Fixes #5331. Let's say we allocate 100 objects, and then free the first 50 of them. If you look at the entire thread, the `retained-native-allocations` view will show the allocation samples for the second 50 objects (the ones that weren't freed). If you look at just the part of the thread where you allocated, you will see allocation samples for all 100 objects (because none were freed in that part of the thread). How do you get the stack category for each of the 100 allocation samples, especially if you've applied transforms or a JS-only filter? You need to look at the stack of the raw nativeAllocations table. You cannot look at the `retained-native-allocations` of the entire thread, because that table doesn't have the stacks for the first 50 objects - it removes those samples because it knows that those objects have been deallocated. This commit makes it so that the unfilteredCtssSamples are just the raw allocation tables, when an allocation-related call tree summary strategy is selected. And it makes it so that `extractSamplesLikeTable` always returns a table that has compatible indexes to the `extractUnfilteredCtssSamples`, just with some samples having a null stack. Now the unfilteredCtssSamples can be used to look up the stack, and therefore the original category of a sample, based on the sample index in the filtered (or preview-filtered) CTSS samples table. We should not create a call tree just based on the unfilteredCtssSamples because these samples have not been filtered according to the strategy.
Hopefully nobody was relying on the fact that the sample data in window.filteredThread was taking the preview selection into account.
I think we were clipping off some rows if there were user timing marker rows.
Stop storing them in the file, compute them at runtime. This reduces profile sizes.
Make sure the start and end grips look like button, make the outbound of the selection even dimmer and add border to make the selection super obvious.
We can use CSS variables whose values are extracted and put into SVG filters that we can then apply on top of the icon we use, in CSS. The time-slice zoom icon takes advantage of this new capability to adapt to High Contrast Mode.
Make sure the start and end grips look like button, make the outbound of the selection even dimmer and add border to make the selection super obvious.
With this commit, we stop storing the stackTable's category and subcategory columns in the file, and compute them at runtime instead. This reduces profile sizes. The upgrader tries to keep the profile https://share.firefox.dev/3ClJLcK from issue #5254 working, which is missing the frameTable's category + subcategory columns, but mostly just because it's easy. This profile is missing other columns too, so it may stop working in the future as we make other changes and start relying on the other columns that the profile is missing.
This avoids a circular dependency between cpu.js, process-profile.js and profile-data.js, because in the next commit I want to use a function from cpu.js in profile-data.js.
It looks like I forgot to update this comment when I changed this function to no longer use the interval.
This makes it clearer that, with time-based CPU deltas, you can have profiles that never hit 100% CPU usage.
[Production](https://share.firefox.dev/4ayKCDQ) | [Deploy preview](https://deploy-preview-5288--perf-html.netlify.app/public/62r9rwwtyrgzj8z3vc40jafaerxcq1gm7zj0y5g/calltree/?globalTrackOrder=80w576&hiddenGlobalTracks=1w46w8&hiddenLocalTracksByPid=1260-0wyb~9280-0w8~10132-0wk~10152-0wk~10120-0wk~8708-0wp~9508-0wx1~9812-0wx4&localTrackOrderByPid=1260-ya0wxgybxiwy9xh~9280-80w7~10132-k0wj~10152-k0wj~10120-k0wj~8708-p0w57wo6~9508-x2x30wx1~9812-x50wx4&range=m1224&thread=0&v=10) `processThreadCPUDelta` makes it so that the `threadCPUDelta` column doesn't have any null values. However, the type of the `threadCPUDelta` column still looks as if values could be null. Now that we have a derived `Thread` type, we can change the type so that it expresses that the values can't be null. So we move the call to `processThreadCPUDelta` a bit earlier in the derivation pipeline so that all the types work out. Even better, we can compute CPU ratios at this point, and not even store the "clamped and non-null cpu delta" values. And since we're doing that, I'm changing the behavior for null cpu delta values: Rather than using the closest non-null cpu delta and spreading it over a potentially-different sample time delta, just set the cpu ratio to 1. The only known cause of null values (other than the first value, which we ignore) is the base profiler, and we want CPU percentage to be reported as 100% during the base profiler time. Fixes #4072.
Co-authored-by: Francesco Lodolo [:flod] <flod+pontoon@mozilla.com> (it)
Co-authored-by: Jim Spentzos <jimspentzos2000@gmail.com> (el)
Co-authored-by: Théo Chevalier <theochevalier@pm.me> (fr)
Co-authored-by: Pin-guang Chen <petercpg@mail.moztw.org> (zh-TW)
Updated locales: be, de, el, en-CA, en-GB, es-CL, fr, fur, fy-NL, ia, it, kab, nl, pt-BR, ru, sv-SE, tr, uk, zh-CN, zh-TW.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates:
[Julien Wajsberg] Some more small refactorings (#5320)
[Markus Stange] Pass the correct sample index offset to getTimingsForCallNodeIndex for the flame graph tooltip. (#5328)
[Nisarg Jhaveri] Update docs to include Android Studio/Simpleperf trace file support (#5309)
[Markus Stange] Don't pass the preview filtered thread to getTimingsForPath/CallNodeIndex. (#5329)
[Nazım Can Altınova] Add a "Sample timestamp" field to the sample tooltip in timeline (#5322)
[Markus Stange] Reduce confusion between call tree summary strategy aware samples and regular samples (#5330)
[Markus Stange] Rename this getCounter selector to getCounters. (#5337)
[Markus Stange] Make sample indexes compatible between the unfiltered and (preview) filtered call tree summary strategy samples when using an allocation strat>
[Markus Stange] Remove some code that uses the preview filtered thread (#5336)
[Markus Stange] Remove getMarkerSchemaName special cases - look up marker schemas from data.type and nothing else (#5293)
[Markus Stange] Remove the makeProfileSerializable step - make the raw in-memory profile match the format that's stored in the file (#5287)
[Nicolas Chevobbe] Adapt FilterNavigatorBar to High Contrast Mode. (#5257)
[Nicolas Chevobbe] Adapt Tracks to High Contrast Mode. (#5252)
[Markus Stange] Adjust string index fields in markers when merging threads (#5344)
[Theodoros Nikolaou] Localize title and aria label in ProfileName (#5345)
[Julien Wajsberg] Adapt time-slice selection in High Contrast Mode. (#5259)
[Markus Stange] Make stackTable (sub)category derived data (#5342)
[Markus Stange] Compute cpuRatio values when computing the derived thread (#5288)
[Nazım Can Altınova] Add a context menu item to open the JS scripts in DevTools debugger (#5295)
Also thanks to our localizers:
el: Jim Spentzos
fr: Théo Chevalier
it: Francesco Lodolo [:flod]
zh-TW: Pin-guang Chen