Reduce some megamorphism in nodes #51380
Conversation
typescript-bot
added
Author: Team
For Uncommitted Bug
|
@typescript-bot perf test faster |
|
Heya @rbuckton, I've started to run the abridged perf test suite on this PR at 976cced. You can monitor the build here. Update: The results are in! |
|
@rbuckton Here they are:Comparison Report - main..51380
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Megamorphism? |
Megamorphic is a term used by V8 to indicate an inline cache (IC) for a property is too polymorphic and can no longer be optimized to a fast property lookup. There's a good article on V8 polymorphism here: https://mrale.ph/blog/2015/01/11/whats-up-with-monomorphism.html |
|
so does it improve? |
|
@rbuckton Ah, thanks. I thought it was referring to something like polymorphism in the sense of, e.g. generic functions. Wouldn’t have guessed “V8 implementation details” |
It's too early to say. I am still experimenting. |
rbuckton
force-pushed
the
reduce-megamorphism
branch
from
976cced
to
5dc93c4
Compare
|
@typescript-bot perf test |
|
Heya @rbuckton, I've started to run the perf test suite on this PR at 5dc93c4. You can monitor the build here. Update: The results are in! |
|
@rbuckton Here they are:
CompilerComparison Report - main..51380
System
Hosts
Scenarios
TSServerComparison Report - main..51380
System
Hosts
Scenarios
StartupComparison Report - main..51380
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rbuckton
force-pushed
the
reduce-megamorphism
branch
from
5dc93c4
to
b669700
Compare
|
@typescript-bot perf test faster |
|
Heya @rbuckton, I've started to run the abridged perf test suite on this PR at b669700. You can monitor the build here. Update: The results are in! |
|
@rbuckton Here they are:Comparison Report - main..51380
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
I wouldn't have suspected quite such a steep increase in memory usage (and I almost glanced over it at first because there is no red icon shown in the memory usage row). |
|
that's a little strange, if you let all node has the same shape (e.g. same property set), aren't they get optimized to some "hidden class" to save memory? or is it caused by the new |
|
I was very surprised that the memory would increase so much, just by adding a few properties. |
|
I’m thinking the memory usage goes up because V8 is keeping more object shapes and associated JIT’d code in memory. My understanding of the inline cache mechanism was that if the engine knows the shape of an object, it can optimize property lookups down to a quick typecheck + some pointer arithmetic, which means different machine code for different object shapes. So if megamorphism is detected, it adds a layer of indirection that lets it use the same machine code for multiple object shapes (though still faster than a full property lookup; something like an inside-out vtable), which reduces memory pressure at the cost of performance. |
|
but here is not adding megamorphism, but reducing it. afaic the new properties added are intended to make all the Nodes have the same shape, therefore the same optimization can apply. |
|
Maybe v8 didn't emit native code (JIT) for them before because there were too many shapes? But either way, I'm surprised for up to 20% more memory. |
|
The increase in memory is totally expected and has nothing to do with ICs/JIT/V8; hidden classes are obtained using a class transition graph so they are nicely shared if an object has reached a certain shape using the same transition path though that graph. The memory increase is due to how there's now dedicated objects allocated for certain pieces of data. Previously their properties were stored on one object, but setting these properties in various orders as the program is executing means that several hidden class transitions are taken, resulting in different hidden classes and therefore resulting in megamorphic accesses for property reads that have observed more than four hidden classes (2-4 is called polymorphic). As an experiment, this PR opts to have only singular object shapes to achieve monomorphism, but this requires that all properties are assigned up front during creation of the objects. Since some properties are only used for a small subset of nodes, this PR currently experiments with storing some properties on dedicated objects to avoid having to always store them in the node. This allows for them to be allocated on-demand (which is good, as it saves memory when not needed) it does mean that once that object is needed we're now paying for the extra allocation, indirection and memory usage. I'd recommend this article that goes into far more detail on this subject. My intention wasn't to blow up this thread with discussions about memory usage; this PR is clearly an experiment where various approaches are attempted to see their impact and to evaluate the trade-offs. |
I know. If you re-read my comment, I speculated that the presence of megamorphism reduces memory pressure and my guess was that that was due to being able to keep more optimized code in memory (fewer de-opts) in the non-megamorphic case, but @JoostK’s explanation indeed makes more sense.
I think it’s probably acceptable to discuss memory usage in a PR that significantly increases it. 😅 Optimizing JS can seem like a black art at times, so even the failed experiments can be instructive. btw @rbuckton already linked that article earlier in the thread, which is what prompted all this speculation in the first place. 😉 |
The increase in memory is due to the additional storage space allocated for What I've done so far is validate whether improving v8 map stability would improve performance in general by reducing the polymorphism that results from various parts of the compiler conditionally attaching properties. The best way to achieve this while reducing the overall memory overhead is to limit such extra fields to specific node subtypes, such as moving the internal The other benefit of this is that we can change places in the checker that currently blindly read While the end result may still incur more memory overhead, some of that overhead will be offset by producing fewer v8 maps and fewer function recompilations due to deopts, which in turn means fewer GC pauses. In most cases, optimizing for monomorphism doesn't give great returns. Especially for the compiler since so many functions must be polymorphic and branch on |
|
Thanks for the detailed explanation! |
|
FWIW, while there may be some V8-specific details here (and V8 is the relevant target for the Node ecosystem), a lot of this applies to other JIT compilers like SpiderMonkey as well:
|
|
Closing in favor of #51682 |
This is a WIP to investigate the effects some code changes would have on reducing megamorphism in the compiler.