Bug 187477

Summary: [JSC] Embed RegExp into constant buffer in UnlinkedCodeBlock and CodeBlock
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
mark.lam: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews205 for win-future none

Description Yusuke Suzuki 2018-07-09 11:32:21 PDT
[JSC] Embed RegExp into constant buffer in UnlinkedCodeBlock and CodeBlock
Comment 1 Yusuke Suzuki 2018-07-09 11:34:48 PDT
Created attachment 344598 [details]
Patch
Comment 2 EWS Watchlist 2018-07-09 13:52:23 PDT
Comment on attachment 344598 [details]
Patch

Attachment 344598 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8484735

New failing tests:
http/tests/preload/onload_event.html
Comment 3 EWS Watchlist 2018-07-09 13:52:34 PDT
Created attachment 344609 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 4 Mark Lam 2018-07-09 14:07:51 PDT
Comment on attachment 344598 [details]
Patch

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

r=me with suggested change.

> Source/JavaScriptCore/runtime/JSCJSValue.cpp:277
> +            out.print("RegExp: ");
> +            RegExp::dumpToStream(asCell(), out);

If you implement WTF::printInternal (see below), then this reduces to:
    out.print("RegExp: ", jsCast<RegExp*>(asCell()));

> Source/JavaScriptCore/runtime/RegExp.cpp:528
> +void RegExp::dumpToStream(const JSCell* cell, PrintStream& out)

I recommend making this a WTF::printInternal instead so that we can use it with dataLog print streams directly.  Grep for "printInternal(PrintStream&" for examples.
Comment 5 Yusuke Suzuki 2018-07-09 14:21:47 PDT
Comment on attachment 344598 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/runtime/JSCJSValue.cpp:277
>> +            RegExp::dumpToStream(asCell(), out);
> 
> If you implement WTF::printInternal (see below), then this reduces to:
>     out.print("RegExp: ", jsCast<RegExp*>(asCell()));

I've changed this code to `out.print("RegExp: ", *jsCast<RegExp*>(asCell())));`. It works since we override dumpToStream in RegExp.

>> Source/JavaScriptCore/runtime/RegExp.cpp:528
>> +void RegExp::dumpToStream(const JSCell* cell, PrintStream& out)
> 
> I recommend making this a WTF::printInternal instead so that we can use it with dataLog print streams directly.  Grep for "printInternal(PrintStream&" for examples.

We override RegExp::dumpToStream, which is well utilized in JSCell ClassInfo mechanism!
Comment 6 Yusuke Suzuki 2018-07-09 14:29:46 PDT
Committed r233657: <https://trac.webkit.org/changeset/233657>
Comment 7 Radar WebKit Bug Importer 2018-07-09 14:31:12 PDT
<rdar://problem/41990301>