Bug 157297 – Web Inspector: console.assert should do far less work when the assertion is true

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

* SUMMARY
console.assert should do far less work when the assertion is true.

* TEST
<script>
(function() {    
    function bench(name, f, arg) {
        var warmup = f();
        var start = Date.now();
        var result = f();
        var end = Date.now();
        console.log(name, (end-start)+"ms");
    }

    bench("console.assert...", function() {
        for (var i = 0; i < 1e6; ++i)
            console.assert(true, "message");
    });
})();
</script>

* RESULTS
• Safari - console.assert... – "119ms"
• Chrome - console.assert... 22ms
• Firefox - console.assert... 44ms

We do a lot of work in the case where the assert doesn't fire. After a quick fix:
• Safari - console.assert... – "9ms"

[Radar WebKit Bug Importer]

<rdar://problem/26056556>

[Joseph Pecoraro]

Created attachment 277972 [details]
[PATCH] Proposed Fix

We could make assert an intrinsic in the future, but I'm not sure it really needs it. This alone was a 10x speedup.

[Timothy Hatcher]

Comment on attachment 277972 [details]
[PATCH] Proposed Fix

Nice!

[WebKit Commit Bot]

Comment on attachment 277972 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 277972

Committed r200371: <http://trac.webkit.org/changeset/200371>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 277972 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=277972&action=review

> Source/JavaScriptCore/runtime/ConsoleClient.h:53
> +    void assertion(ExecState*, RefPtr<Inspector::ScriptArguments>&&);

The right name for this seems to be "assertionFailed".

Longer term, really puzzled about the fact that ScriptArguments is a reference counted object.