New issue
Advanced search Search tips
Starred by 1 user
Status: WontFix
Owner:
Closed: Oct 30
Cc:
Components:
HW: All
OS: All
Priority: 2
Type: Bug

Blocking:
issue 6936



Sign in to add a comment
PREMONOMORPHIC to PREMONOMORPHIC misses in chai test
Project Member Reported by bmeu...@chromium.org, Oct 27 Back to list
In the chai test of the web-tooling-benchmark we see a lot of these really odd .->. misses:

StoreIC (.->.) at *chainableMethodGetter wtb-chai.js:28717:26 call (map 0x32a102689731)
StoreIC (.->.) at *chainableMethodGetter wtb-chai.js:28718:27 apply (map 0x32a1026897d1)
StoreIC (.->.) at *chainableMethodGetter wtb-chai.js:28717:26 call (map 0x32a102689a01)
StoreIC (.->.) at *chainableMethodGetter wtb-chai.js:28718:27 apply (map 0x32a102689aa1)
StoreIC (.->.) at *chainableMethodGetter wtb-chai.js:28717:26 call (map 0x32a102689cd1)
StoreIC (.->.) at *chainableMethodGetter wtb-chai.js:28718:27 apply (map 0x32a102689d71)
StoreIC (.->.) at *chainableMethodGetter wtb-chai.js:28717:26 call (map 0x32a102689fa1)
StoreIC (.->.) at *chainableMethodGetter wtb-chai.js:28718:27 apply (map 0x32a10268a041)

There seems to be something wrong here.
 
Cc: bmeu...@chromium.org cbruni@chromium.org
Labels: -Priority-1 Priority-2
Owner: jkummerow@chromium.org
Status: Assigned
Ok, bisected it to https://chromium-review.googlesource.com/622147 which introduces an optimization that we trigger here. Simplified repro:

===========================
function foo(o) {
  o = Object.create(o);
  o.x = "x";
  return o;
}

foo({});
foo({});
foo({});
foo({});
foo({});
foo({});
===========================

This just misses and stays in PREMONOMORPHIC state instead of going MEGAMORPHIC. Reverting the CL properly transitions the STORE_IC to MEGAMORPHIC state.
Status: WontFix
I agree that seeing .->. misses seems odd, but I don't see a good alternative in the short term.

The facts at play here are:
(1) "o = Object.create(p)" must create a new map (for o) for every p that comes in, because we store the prototype in the map.
(2) Transitioning StoreICs don't create a handler for transitions that are being taken for the first time, to avoid creation of handlers that might not be used again. Without a handler, there is no reasonable way for the IC to make progress through its state transitions, so it simply remains where it is.

We could try to come up with heuristics for transitioning such an IC (that only ever sees fresh maps where it has to create new transitions) into MEGAMORPHIC state eventually, but that would only make --trace-ic output look less weird (trading .->. for N->N); it would not yield any performance benefit (as the MEGAMORPHIC IC would still have to miss, call the runtime, create a new map transition, create a handler to populate the stub cache with -- so in fact it would be a bit slower, I'm seeing a ~5% regression on the benchmark below).

If we really wanted to make an impact here, we would probably have to propagate a bit of information like "this object has shown up in many megamorphic ICs" back to its creation site (i.e. the Object.create() call in this case) and cause that creation site to produce dictionary-mode objects instead. Since the prototype object is different every time, and the object is subsequently used as a prototype, we would still create two fresh maps per function call, but that would be down from four, and the two StoreICs could add dictionary properties without missing. But that sounds like a pretty big and possibly controversial project (needs AllocationMementos or a similar mechanism).

[Side note: I wonder whether chai could be improved. Creating a new prototype object every time a getter is called sounds really expensive. Reference:
https://github.com/chaijs/chai/blob/master/lib/chai/utils/addChainableMethod.js
]

Benchmark to show that going to MEGAMORPHIC wouldn't help performance:

const kForceMegamorphic = true;
function foo(o) {
  o.x = true;
  return o;
}

if (kForceMegamorphic) {
  foo({a: 1});
  foo({a: 1});
  foo({b: 1});
  foo({b: 1});
  foo({c: 1});
  foo({c: 1});
  foo({d: 1});
  foo({d: 1});
  foo({e: 1});
  foo({e: 1});
}

var t1 = Date.now();
var result = true;
for (var i = 0; i < 1000000; i++) {
  result = result && foo(Object.create({})).x;
}
var t2 = Date.now();
print("Took " + (t2 - t1) + " ms." + (result ? "" : ""));

Thanks for the investigation. One thing that comes to mind tho: Doesn't the transition to PREMONOMORPHIC reset the feedback ticks? If so that would influence tier up decisions.
There is only one 0->. transition at the beginning. The ".->." misses don't transition or reset anything.
Sign in to add a comment