1494504 - IonSetPropertyIC stubs transition to Generic too quickly on GDocs

Status: RESOLVED DUPLICATE of bug 1494537

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[Bas Schouten (:bas.schouten)]

From what Kannan has been telling me this is bad, it shouldn't be hit that much in hot code. What I'm actually seeing is it's being called 20k+ times per key stroke.. I'm attempting to understand why because this seems bad.

Comment 1

[Bas Schouten (:bas.schouten)]

This is happening because the ICState's mode for the relevant stubs is in 'generic' I'm trying to add instrumentation to discover why.

Comment 2

[Bas Schouten (:bas.schouten)]

I made an attempt in my instrumentation to locate the point of the transition to generic, one place I found my instrumentation outputted the following:

Cannot attach stub.
Calling IonSetPropertyIC::update count: 1089001
Cannot attach stub.
Calling IonSetPropertyIC::update count: 1089002
Cannot attach stub.
Calling IonSetPropertyIC::update count: 1089003
Cannot attach stub.
Calling IonSetPropertyIC::update count: 1089004
Cannot attach stub.
Calling IonSetPropertyIC::update count: 1089005
Cannot attach stub.
Calling IonSetPropertyIC::update count: 1089006
Calling IonSetPropertyIC::update count: 1089007
Calling IonSetPropertyIC::update count: 1089008
Calling IonSetPropertyIC::update count: 1089009
Calling IonSetPropertyIC::update count: 1089010
Calling IonSetPropertyIC::update count: 1089011
Discarding stubs
Discarding stubs
Calling IonSetPropertyIC::update count: 1089012
Discarding stubs
Discarding stubs
Transitioning state to generic!
Discarding stubs
Calling IonSetPropertyIC::update count: 1089013


This is a little off, it's attaching 6 stubs succesfully, then it discards them (presumably this was a transition to megamorphic which I am not tracking in this build), then it attaches one more stub and right away transitions to generic, as if the stub count was never reset, however as far as I can tell the stub count should be reset in IonIC::discardStubs because of trackUnlinkedAllStubs being called.

Comment 3

[Bas Schouten (:bas.schouten)]

So the above log has some sad mixup of different objects. Further instrumentation shows that the vast majority of these 20k+ update calls are coming from a single IonSetPropertyIC object, that at some point early in its life experiences a large amount of failures attaching the stub, causing the state object to transaition into generic mode and everything is screwed from there on.

Comment 4

[Bas Schouten (:bas.schouten)]

So the reason these stubs are failing to attach is because inside SetPropIRGenerator::tryAttachAddSlotStub the check for nameOrSymbol fails.

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

I'm re-summarying this bug because we have a family that are very hard to tell apart at a glance. 

I'd like to use this bug to track and investigate the fact that our stubs are transitioning to generic mode too quickly, meaning that future potential attachments never happen. 

(Bas Schouten (:bas.schouten) from comment #2)
> 
> This is a little off, it's attaching 6 stubs succesfully, then it discards
> them (presumably this was a transition to megamorphic which I am not
> tracking in this build), then it attaches one more stub and right away
> transitions to generic, as if the stub count was never reset, however as far
> as I can tell the stub count should be reset in IonIC::discardStubs because
> of trackUnlinkedAllStubs being called.

Comment 6

[Nicolas B. Pierron [:nbp]]

Matthew, are you going to take this bug to get it fixed for Firefox 64 or do you know if someone else might be able to?

Comment 7

[Ted Campbell [:tcampbell]]

Bug 1494537 should fix this, but we'll need to verify.

Comment 8

[Matthew Gaudet (he/him) [:mgaudet]]

As Ted says, this will likely get fixed as a side effect of Bug 1494537. However, I think there's also some relevance to Bug 1495519.

Comment 9

[Kannan Vijayan [:djvj]]

Closing this as a dup of 1494537, because the specific case this arises out of has been addressed for the use case that prompted this investigation.