Perplexed by aborted optimizations for reason "Unsupported phi use of const variable"

[Scott Hulbert]

I am seeing lines such as:

    [compiling method 0x39790c6f2379 <JS Function getVirtuals (SharedFunctionInfo 0x2e54cd11ffa9)> using Crankshaft]

    [aborted optimizing 0x39790c6f2379 <JS Function getVirtuals (SharedFunctionInfo 0x2e54cd11ffa9)> because: Unsupported phi use of const variable]

In spite of this code not using `const` declarations (nor the method that invokes it). I even tried replacing `const` with `var` in all `node_modules` files on a smaller project and saw the same issue.

This leads me to believe the explanation is inaccurate and the aborted optimization is not directly related to use of ES6 const?

Here's the above function that won't optimize, though it's technically already copied into our codebase and I've tried rewriting it a few different ways: https://github.com/tgriesser/bookshelf/blob/ca08ec0850f758ec5610e3c543db5a276026624e/plugins/virtuals.js#L92-L100

Thanks,

Scott Hulbert

[Jakob Kummerow]

This is indeed surprising.

Crankshaft predates ES6, so this has nothing to do with ES6 const. That said, I don't see why this function would bail out; and in a quick repro attempt it doesn't. Do you have an example I can run that reproduces this behavior?

Also, is this actually a performance problem for you? Crankshaft aborts compilation in some cases by design, often there's no need to worry about it.

--
--
v8-users mailing list
v8-u...@googlegroups.com
http://groups.google.com/group/v8-users
---
You received this message because you are subscribed to the Google Groups "v8-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to v8-users+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Scott Hulbert]

Jakob, here is a reproducible example (https://github.com/hulbert/bookshelf-sample-v8). I’m running this with node 5.6.0 on OS X (10.10.4).

    git clone g...@github.com:hulbert/bookshelf-sample-v8.git

    npm install

    node --trace-opt index.js | grep ‘getVirtuals'

outputs:

    [marking 0x185d96391a91 <JS Function getVirtuals (SharedFunctionInfo 0x185d96347581)> for recompilation, reason: small function, ICs with typeinfo: 2/4 (50%), generic ICs: 0/4 (0%)]

    [compiling method 0x185d96391a91 <JS Function getVirtuals (SharedFunctionInfo 0x185d96347581)> using Crankshaft]

    [aborted optimizing 0x185d96391a91 <JS Function getVirtuals (SharedFunctionInfo 0x185d96347581)> because: Unsupported phi use of const variable]

    [disabled optimization for 0x185d96347581 <SharedFunctionInfo getVirtuals>, reason: Unsupported phi use of const variable]

> Also, is this actually a performance problem for you? Crankshaft aborts compilation in some cases by design, often there's no need to worry about it.

This code is part of serialization in a web app and we’re trying to eek some extra performance out of it when serializing really large numbers of objects populated by an ORM. There are a couple other cases like this (for example I believe `_toJSON` in my example isn’t optimized because of the same issue), but `getVirtuals` is the most isolated case IMO. 

I am also hoping to get a better understanding of the "Unsupported phi use of const variable” reason as there is very little information about it available on the web. I’d like to contribute details to the list of V8 bailout reasons (https://github.com/vhf/v8-bailout-reasons).

Thanks,

Scott Hulbert

You received this message because you are subscribed to a topic in the Google Groups "v8-users" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/v8-users/hsUrt4I2D98/unsubscribe.
To unsubscribe from this group and all its topics, send an email to v8-users+u...@googlegroups.com.

[Ben Noordhuis]

On Mon, Feb 29, 2016 at 1:23 PM, Jakob Kummerow <jkum...@chromium.org> wrote:
> This is indeed surprising.
> Crankshaft predates ES6, so this has nothing to do with ES6 const. That
> said, I don't see why this function would bail out; and in a quick repro
> attempt it doesn't. Do you have an example I can run that reproduces this
> behavior?
>
> Also, is this actually a performance problem for you? Crankshaft aborts
> compilation in some cases by design, often there's no need to worry about
> it.

If it helps, see [0] - we had to revert some changes to the url module
because of this issue. That's with V8 4.8.271.17.

The benchmarks are in [1]. They normally run in a child process but
you can run an individual benchmark directly with `./node
benchmark/url/url-parse.js type=one n=25e4`.

[0] https://github.com/nodejs/node/pull/5300
[1] https://github.com/nodejs/node/blob/v5.7.0/benchmark/url/url-parse.js

[Scott Hulbert]

In interest of providing extra information, my installation of Node 5.6.0 is using V8 version 4.6.85.31.

I’ve also committed the `node_modules` directory for my sample project and replaced all the instances of `const` I could find via search with `var`: https://github.com/hulbert/bookshelf-sample-v8/commits/modules-directory

Results are the same.

Best,

Scott Hulbert

--
--
v8-users mailing list
v8-u...@googlegroups.com
http://groups.google.com/group/v8-users
---

You received this message because you are subscribed to a topic in the Google Groups "v8-users" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/v8-users/hsUrt4I2D98/unsubscribe.
To unsubscribe from this group and all its topics, send an email to v8-users+u...@googlegroups.com.

[Ali Sheikh]

I found the bailout reason message to be confusing too. What it really is trying to check is if there is a 'unsupport Phi use of a constant HOLE value' (Here's the bailout check: [1]).

Changing the `let virtualName` to `var virtualName` in the for-in loop in your code would fix the problem with the bailout:

function getVirtuals(model) {
    var virtuals
    var attrs = {}
    if (virtuals = model.virtuals) {
        for (var virtualName in virtuals) {
            attrs[virtualName] = getVirtual(model, virtualName)
        }
    }
    return attrs
}

As for the explanation, someone who understands Hydrogen IR better would have to explain that. I don't understand why the culprit phi value (v88) is there to begin with. It doesn't seem to have real uses. For convenience, here's a link to the Hydrogen IR for the `getVirtuals` function: [2]. 

[1] https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/crankshaft/hydrogen.cc&sq=package:chromium&type=cs&rcl=1456748799&l=4005

[2] https://gist.github.com/ofrobots/6d8d391a047ee8f0766a

You received this message because you are subscribed to the Google Groups "v8-users" group.

To unsubscribe from this group and stop receiving emails from it, send an email to v8-users+u...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Ali

[Jakob Kummerow]

On Mon, Feb 29, 2016 at 9:31 PM, Scott Hulbert <sahu...@gmail.com> wrote:

Jakob, here is a reproducible example (https://github.com/hulbert/bookshelf-sample-v8). I’m running this with node 5.6.0 on OS X (10.10.4).

That example explains a lot. Here, you're using "for (let virtualName in virtuals) {", whereas the original example used "var". "const" and "let" are similar in that they're both block-scoped, which isn't properly supported by Crankshaft. The error message wasn't updated to reflect this.

Since Crankshaft is deprecated and scoping is nasty, I'm hesitant to spend time on adding better support for let/const. Instead, what we should do is use Turbofan in cases like this --> https://codereview.chromium.org/1751873002/ (which as a drive-by fix updates the bailout message).