Fix isObject deopt

[chancancode]

This deopts the ChainNode constructor because inlining

Fixes #14113

[stefanpenner]

👍

[jdalton]

@chancancode

Can you explain the steps you performed to get the de-opt data?

Could it be that the method is returning potentially a different type?
In the old code if obj was undefined it would return undefined instead of boolean.

Would changing it to:

return !!obj && typeof obj === 'object';

or

return obj !== null && typeof obj === 'object';

have the same effect?

[krisselden]

I can explain, typeof obj is a generic check, coercion to boolean is type specific, if a value you coerce can be anything, like in the above code and in a hot path, it may have only compiled some of the coercions. In the above example it had always be an object, so it only had compiled the falsy conversion for object.

We should try return obj !== null && typeof obj === 'object'; as that first check maybe generic, that it doesn't matter if it becomes something else later.

I don't know how !! doesn't require knowing the type specific conversion to false, then true.

[krisselden]

@jdalton in these cases it is being inlined, so it knows it needs a boolean, it isn't the return type that matters, it is type being converted to boolean.

Both CompareObjectEqAndBranch (the example you gave with obj !== null && typeof obj === 'object') and TypeofIsAndBranch are generic operations and resilient to non objects being passed into isObject(obj).

The issue is this conversions to booleans are typed, so they deopt when they hit another one of these cases, likely will compile with another one of the cases https://cs.chromium.org/chromium/src/v8/src/code-stubs.cc?q=SpecObject&sq=package:chromium&dr=C&l=5534

[krisselden]

More detail here: https://gist.github.com/krisselden/f123f6ef3c9246cbbcd98a1958bbd1eb

It will deopt to a generic conversion, and this is an optimization that speeds up the non generic case. In our case, it was in a generic code path that is hot.

In some cases where you don't actually expect a variety and this is guarding the exceptional, maybe the falsy conversion is faster than typeof? Deopts can be quite fast, and if the trade off is faster performance later than maybe it is better to let it deopt.

The problem is how costly a deopt is depends on a number of factors. Is this area of the code hot and recursive? Is this inlined into a function that is optimized on the call stack? Is this initial render with lots of GC pressure? Could de-optimizing here worsen the next round of optimizations, less things inlined, etc.

A micro benchmark can absorb some rounds of de-opts to settle on something nice. During initial render, it can be hard to leverage benefit from the JIT in the way that a micro benchmark does.

[jdalton]

@krisselden

Cool. Sounds kinda brittle to fret over.
Is this code only ever executed in a locked version of v8?

[krisselden]

Often, something like uglify undoes it anyway.

[krisselden]

Like the UMD module parens hack, that is 2 extra characters you didn't need.