Skip to content

Deploy Mar 3, 2025#5389

Merged
julienw merged 32 commits into
productionfrom
main
Mar 3, 2025
Merged

Deploy Mar 3, 2025#5389
julienw merged 32 commits into
productionfrom
main

Conversation

@julienw

@julienw julienw commented Mar 3, 2025

Copy link
Copy Markdown
Contributor

[Julien Wajsberg] Update node to v22 (#5378)
[Markus Stange] Speed up createCallNodeTable by 2.3x (#5248)
[Markus Stange] Remove frameTable.implementation from the processed format. (#5370)
[Florian Quèze] Show size units in the timeline for profiles where profile.meta.sampleUnits.time is "bytes". (#5364)
[Sean Kim] Report nsIRequest::status (nsresult) in the marker (#5375)
[Markus Stange] Change the marker schema to accept a description and get rid of the notion of static fields (#5385)

Also thanks to our localizers:
en-CA: Paul
tr: Grk

Firefox Profiler [bot] and others added 30 commits February 20, 2025 08:03
* Update node to v22

* Update tests
Co-authored-by: Paul <seburo3@gmail.com> (en-CA)
…umns until we know the final size and order of the call node table.
Co-authored-by: Grk <gu0townkg@relay.firefox.com> (tr)
Fixes #3713.

A frame's "implementation" was used to differentiate various JIT tiers.
We now have subcategory information which fulfills this job more
accurately.

So we can get rid of this extra data and reduce profile sizes.

This also lets us remove a lot of code.

I have a 3.1GB profile which contains 200MB of `,null` for the
implementation column. With this change that profile becomes 200MB
smaller.

---

This PR leaves the Gecko format unchanged. As a follow-up we should
write a Gecko patch and also bump the Gecko version, but there's no rush
to do so; making this change only for the processed format achieves a
large part of the benefits.
This replaces some lesser used dependencies (the ones only used in some
scripts we don't use every day) by running `npx` to install them on the
fly. Note that Yarn v2 has `yarn dlx` but we don't have that with Yarn
v1.

On my machine this reduces `node_modules` by 142 MB, that is more than
10%. Indeed a lot of the transitive dependencies were outdated and
therefore duplicated from the more recent ones.

Also these transitive dependencies had security advisories. Good
riddance :-)
Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com>
Co-authored-by: Nazım Can Altınova <canaltinova@gmail.com>
…rsion 33.

When the test was initially added, the markerSchema was missing from the profile.
That was invalid because version 33 is the version which started requiring that
the markerSchema field is present.

In #4352 we added the missing markerSchema because the upgrader for
version 44 caught the fact that it was missing, because it (rightfully) assumes
that it's present.

However, since this is a profile of a fixed version, we shouldn't attempt to
share code with other tests - anything that's shared will usually follow the
most recent format version, whereas this test must make sure the profile it
creates complies with the hardcoded fixed version.
Use an optional description field instead.

This causes less trouble with our Flow types. It's also a clearer API.
And it will make it easier to switch marker.data from being an object
to it being an array, in field order, because we'll be able to say that
there should be one array element per field.
…otion of static fields (#5385)

In a marker schema, the `data` array can currently hold both "dynamic"
and "static" fields. Dynamic fields get their value from each marker,
static fields have a single fixed value.

In the years since the marker schema has been available, the only thing
we've used "static" fields for is a "description" field.

So I think we should just have an actual description field which is
separate from schema.data. And we should remove generic "static" fields.
This simplifies the format, and it also simplifies our code because we
can iterate over the fields without having to check whether a field is a
static or a dynamic field - Flow was requiring extra verbose checks in
this case because the two field types don't have a shared discriminator
property.

This change does make the schema a little less expressive, but nobody
was making use of the extra expressivity.

Also, at some point we'll want to move to a more compact representation
of markers in the profile JSON by not repeating the field keys on every
marker. One way to do this would be to store marker field values in an
array, where each array value maps to the field at the corresponding
index. This representation makes more sense if the fields array only
contains fields that actually get their value from the marker.

This PR only changes the processed format. The Gecko profile format
remains unchanged. We can change it in a follow-up.

On the Gecko side, we currently have an old and [a new
way](https://bugzilla.mozilla.org/show_bug.cgi?id=1869835) to declare
marker schemas. The new way got rid of static fields too and just
accepts [a
description](https://searchfox.org/mozilla-central/rev/f256e50e068136275c1a2aff827aeddc92a75e07/mozglue/baseprofiler/public/BaseProfilerMarkersPrerequisites.h#1089-1091)
(but still outputs the description as a static field).
@julienw julienw merged commit 348fb13 into production Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants