Closed
Bug 1501787
Opened 6 years ago
Closed 5 years ago
Reset entry counters for IC chain when adding a new stub
Categories
(Core :: JavaScript Engine: JIT, enhancement, P3)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: mgaudet, Assigned: mgaudet)
References
Details
Attachments
(1 file)
In Bug 1494473, Comment 11, I described the limitations of reasoning that we have with IC entry counters. Essentially, because we add stubs at the beginning of the chain, reasoning about relative frequency is challenging. Quoting that comment (edited): > The output for a particular IC chain looks like this: > > box2d.js:174:250 (sub) 1009 -> 772 -> (fb) 2 > > Which is read like this: This sub opcode has two ICs attached. The first IC was > entered 1009 times, the second 772 and the fallback stub was hit twice. > > - We cannot however say necessarily that the first IC failed to provide a value > 771 times. > > Since new ICs are added at the front, it is possible that there was a phase > transition that happened, and the first 771 requests were served by the first > IC, followed by a transition to a new type and addition of a new IC, and > subsequently 1009 queries were served by the newly added IC. In Bug 1494473, Comment 12, I proposed a solution: Whenever a new IC is added to the front of the chain, walk the rest of the ICs, and reset the entry counter to 1. This restores the ability to reason about the execution of the chain. If the above chain statistics had been found with that policy in place, we -would- be able to conclude that the first stub provided a value for only 237 out of 1009 queries, which would be the queries since the chain missed and hit the fallback path that produced the frontmost stub. Similarly, when stubs are collected, you'd reset the chain counters. My initial hesitance for doing this was that I wanted a more global viewpoint of the execution. However, I think overall, it's more important to have an accurate mechanism of reasoning based on the IC chain.
Assignee | ||
Comment 1•6 years ago
|
||
Re-reading the code to addNewStub [1], we are actually inserting new stubs preceding the fallback, but after all the existing stubs. This makes this bug invalid. [1]: https://searchfox.org/mozilla-central/rev/17f55aee76b7c4610a974cffd3453454e0c8de7b/js/src/jit/BaselineIC.h#705-713
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
Assignee | ||
Comment 2•6 years ago
|
||
The reasoning in Comment 1 is wrong, but after some discussion about interpreting IC chains entry counters, I came to the conclusion that we still do want to do this.
Assignee: nobody → mgaudet
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Assignee | ||
Comment 3•5 years ago
|
||
Resetting the entered count on an IC stub chain makes interpretation of the counter values easier. For example, if we see an IC chain like this A (1000) -> B(800) -> C (400) -> D (200) -> FB (100) We can say that there have been 100 cases not handled by chain, (and we did not attach new stubs for those cases), and B handled the most (400) queries to the IC chain.
Pushed by mgaudet@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c376952fc919 Reset entered counts on stub attachment r=djvj
Comment 5•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c376952fc919
Status: REOPENED → RESOLVED
Closed: 6 years ago → 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in
before you can comment on or make changes to this bug.
Description
•