V8 compile: cacheAccepted = false

[isaksky]

Hi, first of all, thank you for what seems like a really great library.

I have a question about how compilation caching is meant to be used. In the first iteration of this loop, the cacheBytes are (understandably) not accepted, yet a working script that can be executed is returned anyway. Since it returns a working script, it makes me think it did a compilation, but I couldn't see how I could populate cacheBytes based on that result. Hence the second call to v8.Compile, with the different arity that has cacheBytes as an out parameter. Is there a better way?

Would it make sense to have a method that that has cacheBytes as a ref parameter, so the function can use it or update it as appropriate?

    var v8 = new V8Runtime();
    var eng = v8.CreateScriptEngine();    
    var cacheBytes = new byte[0];
    
    var code = "console.log('hello'); 5";
    
    for (int i = 0; i < 2; i++) {
        var script = v8.Compile(code, V8CacheKind.Code, cacheBytes, out var cacheAccepted);
        var res = eng.Evaluate(script);
        if (!cacheAccepted) {
            v8.Compile(code, V8CacheKind.Code, out cacheBytes);
        }
        script.Dump("s");
        cacheAccepted.Dump("ca?");
        res.Dump("result");
    }

[ClearScriptLib]

Hi @isaksky,

Is there a better way?

As you've noted, both Compile overloads successfully compile the script. Caching allows you to accelerate re-compilation in case the script was compiled by a different V8 runtime or if the compiled script is no longer available.

Looking at your sample code, we aren't clear on why your first Compile call is to the overload that accepts cache data. As it is the first call, we know that cache data is not yet available, so the other overload seems more appropriate.

Consider this alternative:

byte[] cacheBytes = null;
for (int i = 0; i < 2; i++) {
    V8Script script;
    if (cacheBytes == null) {
        script = v8.Compile(code, V8CacheKind.Code, out cacheBytes);
        if (cacheBytes != null) {
            Console.WriteLine("cache created");
        }
    } else {
        script = v8.Compile(code, V8CacheKind.Code, cacheBytes, out var cacheAccepted);
        if (!cacheAccepted) {
            Console.WriteLine("cache rejected");
            cacheBytes = null;
        }
    }
    Console.WriteLine(eng.Evaluate(script));
}

Here's the output:

cache created
hello
5
hello
5

Would it make sense to have a method that that has cacheBytes as a ref parameter, so the function can use it or update it as appropriate?

Sure; that could be done with a simple extension method:

public static class V8RuntimeExtensions {
    public static V8Script Compile(this V8Runtime v8, string code, ref byte[] cacheBytes) {
        V8Script script;
        if (cacheBytes == null) {
            script = v8.Compile(code, V8CacheKind.Code, out cacheBytes);
            if (cacheBytes != null) {
                Console.WriteLine("cache created");
            }
        } else {
            script = v8.Compile(code, V8CacheKind.Code, cacheBytes, out var cacheAccepted);
            if (!cacheAccepted) {
                Console.WriteLine("cache rejected");
                cacheBytes = null;
            }
        }
        return script;
    }
}

With that extension class in effect, the sample code above can be reduced to this:

byte[] cacheBytes = null;
for (int i = 0; i < 2; i++) {
    Console.WriteLine(eng.Evaluate(v8.Compile(code, ref cacheBytes)));
}

Good luck!

[isaksky]

Looking at your sample code, we aren't clear on why your first Compile call is to the overload that accepts cache data. As it is the first call, we know that cache data is not yet available, so the other overload seems more appropriate.

That is true, the other overload is appropriate if we know there are no cacheBytes. But in this case, I was assuming I did have some bytes from a previous call that I have stored somewhere, but I don't know if they are valid.

In your extension method, it looks like you are failing to update the cacheBytes if stale, non-null cacheBytes are passed in. This means caching would work for one release, and then be blocked for the remainder of your programs lifetime when you update ClearScript, if I'm understanding things correctly. Doesn't that indicate a problem with the API?

It can of course be worked around by doing a check like I do in my code above, and then calling compile again if you were told from the first call that the cacheBytes were not accepted. But why can't you get updated cacheBytes even if the cacheBytes you provided were stale? Aren't the cacheBytes derived from the result - the compiled script, which you get in both overloads?

[ClearScriptLib]

I was assuming I did have some bytes from a previous call that I have stored somewhere, but I don't know if they are valid.

Ah, understood.

In your extension method, it looks like you are failing to update the cacheBytes if stale, non-null cacheBytes are passed in.

We accidentally omitted a line. Sorry about that! Here's the fixed extension class:

public static class V8RuntimeExtensions {
    public static V8Script Compile(this V8Runtime v8, string code, ref byte[] cacheBytes) {
        V8Script script;
        if (cacheBytes == null) {
            script = v8.Compile(code, V8CacheKind.Code, out cacheBytes);
            if (cacheBytes != null) {
                Console.WriteLine("cache created");
            }
        } else {
            script = v8.Compile(code, V8CacheKind.Code, cacheBytes, out var cacheAccepted);
            if (!cacheAccepted) {
                Console.WriteLine("cache rejected");
                cacheBytes = null; // <-- PREVIOUSLY OMITTED
            }
        }
        return script;
    }
}

This still requires two calls to update invalid cache data, but the second call isn't made until it's needed.

But why can't you get updated cacheBytes even if the cacheBytes you provided were stale?

If memory serves, ClearScript's API mirrors that of an old V8 release, where the compilation call could either generate cache data or consume it. V8's API has changed quite a bit in this area – dropping the "parser caching" feature altogether – and now it should be possible to add an API that works as you expect. We'll take a closer look.

Thanks for pointing out this potential improvement!

[isaksky]

That sounds great, thank you for checking!

[ClearScriptLib]

Version 7.4.3 added several new compilation APIs. An example is here.

[isaksky]

Awesome! Thank you.