Closed Bug 1490365 Opened 6 years ago Closed 4 years ago

Consider logging generic mode ICs to CacheIR logs

Categories

(Core :: JavaScript Engine: JIT, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Tracking Status
firefox64 --- affected

People

(Reporter: mgaudet, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 obsolete file)

Right now, once an IC transitions into Mode::Generic, it becomes invisible to our CacheIR logs, which may affect our ability to use them for analysis in seeking opportunities. Seems like they should should show up somewhere.
Blocks: CacheIR
One suggestion Ted made is we could amortize the cost and log every N generic mode hits -- ie, every 1000.
Priority: -- → P3

One of the challenges with our existing IC infrastructure is that once an IC
site goes to Generic mode we can no longer gain any insight about that IC site.

While we can get a feel for the worst case performance degredations through a
profiler showing lots of time being spent in the fallback code, this patch
allows us to count on a per-site basis the number of generic misses.

I'm not requesting review on this just yet because I'm still pondering the
approach.

  • I think it's possible this information could be useful in driving
    heristics later, but I'm not quite there yet.
  • This costs memory for each ICState. Testing with AWSY suggests its
    within the noise range, but in light of memshrink, landing new mem
    usage without consumers seems wrong.

Example output:

{
  "channel":"BaselineGenericStats",
  "location":{
    "filename":"https://docs.google.com/static/document/client/js/2517308900-client_js_prod_kix_core__en_gb.js line 98 > eval",
    "line":808,
    "column":466
  },
  "dump_reason":"Discard Script",
  "entries":[{
    "op":"setprop",
    "pc":316,
    "line":808,
    "column":616,
    "generic":1690
  },
  {
    "op":"setprop",
    "pc":387,
    "line":808,
    "column":648,
    "generic":1690
  }]
},
Assignee: nobody → mgaudet
Status: NEW → ASSIGNED
Attachment #9044314 - Attachment is obsolete: true

Unassigning myself -- not sure I'm going to run with the attached patch.

Assignee: mgaudet → nobody
Status: ASSIGNED → NEW

I think this is no longer necessary post CacheIR health report work, but ni? Caroline to double check the idea isn't interesting anymore.

Flags: needinfo?(ccullen)

For CacheIR health report, when we transition into generic mode we spew what the IC looks at at the time of transition. With this, we also spew the final warm up count of the script it belongs to which gives us an even fuller picture. I believe that covers the general idea of what this bug is looking for.

Flags: needinfo?(ccullen)
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: