Closed Bug 1658866 Opened 4 years ago Closed 3 years ago

Gather various statistics related to strings

Categories

(Core :: JavaScript: GC, task, P3)

task

Tracking

()

RESOLVED FIXED
86 Branch
Tracking Status
firefox86 --- fixed

People

(Reporter: sfink, Assigned: sfink)

References

(Depends on 1 open bug)

Details

Attachments

(4 files)

I've been doing awsy with various stats gathering patches, to understand what we actually do with our strings and explore potential improvements.

There are many parts of my implementation that could be argued. This is just one path.

Can we put more of this code behind some #ifdef (or function that returns a constant based on an #ifdef)?

I know it's only a handful of instructions each time but in general I'm wary of adding overhead to hot paths for something that only a handful of people actually use.

(In reply to Jan de Mooij [:jandem] from comment #5)

Can we put more of this code behind some #ifdef (or function that returns a constant based on an #ifdef)?

I know it's only a handful of instructions each time but in general I'm wary of adding overhead to hot paths for something that only a handful of people actually use.

Usually I am very conservative about that, and gate everything on #ifdefs. Which means most of them bitrot and end up getting removed. I was hoping to be more aggressive this time and measure to make sure I wasn't slowing everything down. That would enable tracking at least some of these values via telemetry (I'd particularly like that for deduplication stats, but they're also some of the cheapest to collect. Especially if I #ifdefed the chars/bytes and only tracked string count by default. Deduplication telemetry would warn us when eg facebook starts running stupid code that makes millions of duplicate strings again.)

But you're right, I don't want to slow things down in the common case for something that's barely used. I'd especially prefer to keep it in C++ code and not JIT code as much as possible. It's tricky, because I can't actually usefully diagram out the magnitude of various string transitions (rope -> extensible -> dependent, for example) without having a base count of the number of ropes allocated, and for that I need JIT support. (And I can't count ropes after the fact, since there will be rope -> dependent transitions that die before the next minor GC.) That's more of an occasional stat that I'd only want in the right places in the code to avoid bitrot, though; I'm fine #ifdefing it.

Actually, the JIT stuff is amenable to environment variables, which are perfect for when I just want to check something in a release build or in a try push. They're not as good for hot C++ paths, though I could pull out the usual excuse of "but it's a well-predicted branch".

(In reply to Steve Fink [:sfink] [:s:] from comment #6)

Usually I am very conservative about that, and gate everything on #ifdefs. Which means most of them bitrot and end up getting removed.

To avoid having #ifdefs everywhere and the bitrot you mentioned, I like the pattern where you have a global function that always returns false at least on non-Nightly builds. That helps avoid bitrot because the code is still "compiled" but any decent compiler should eliminate it as dead code.

Severity: -- → N/A
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f4dae20686a4
Collect and report statistics on string deduplication r=jonco
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch
Regressions: 1683473
Attachment #9169756 - Attachment description: Bug 1658866 - Collect stats on string flattening behavior (whether extensible space is taken advantage of for non-inline strings) and report totals at shutdown → Bug 1658866 - Collect stats on string flattening behavior (whether extensible space is taken advantage of for non-inline strings)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: