Bug 202827

Summary: Canonicalize how we prepare the prototype chain for inline caching
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cgarcia, commit-queue, fpizlo, ggaren, gskachkov, guijemont, keith_miller, mark.lam, mcatanzaro, msaboff, rmorisset, ticaiolima, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=201994
Attachments:
Description Flags
WIP
none
WIP
none
patch
none
patch
ysuzuki: review+
patch for landing
commit-queue: commit-queue-
patch for landing none

Description Saam Barati 2019-10-10 14:53:22 PDT
...
Comment 1 Saam Barati 2019-10-10 19:14:29 PDT
Created attachment 380714 [details]
WIP

it begins
Comment 2 Radar WebKit Bug Importer 2019-10-11 09:51:59 PDT
<rdar://problem/56193919>
Comment 3 Saam Barati 2019-10-11 10:36:54 PDT
Created attachment 380762 [details]
WIP
Comment 4 Saam Barati 2019-10-11 15:47:48 PDT
Created attachment 380796 [details]
patch
Comment 5 Saam Barati 2019-10-11 15:49:34 PDT
Comment on attachment 380796 [details]
patch

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

> Tools/Scripts/run-jsc-stress-tests:491
> +EAGER_OPTIONS = ["--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--thresholdForOMGOptimizeAfterWarmUp=20", "--thresholdForOMGOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "--useEagerCodeBlockJettisonTiming=true", "--repatchBufferingCountdown=0"]

this is helpful to force us to generate IC code more eagerly. This makes my test case repro.
Comment 6 Saam Barati 2019-10-11 16:34:56 PDT
Created attachment 380799 [details]
patch
Comment 7 Yusuke Suzuki 2019-10-11 16:44:09 PDT
Comment on attachment 380796 [details]
patch

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

r=me

> Source/JavaScriptCore/ChangeLog:20
> +        try just defer caching to later later if we encounter a situation where we

/later later/later/s

> Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp:511
> +// Current might be null. Structure can't be null.

Can you insert `ASSERT(structure)` too?

> Source/JavaScriptCore/jit/Repatch.cpp:307
> +                        // Property offsets may have changed due to flattening. We'll cache later.

Sounds fine.

> Source/JavaScriptCore/jit/Repatch.cpp:510
> +                Structure* newStructure = Structure::addPropertyTransitionToExistingStructureConcurrently(structure, ident.impl(), 0, offset);

I like, `static_cast<unsigned>(PropertyAttribute::None)`.
Comment 8 Saam Barati 2019-10-14 08:39:02 PDT
Comment on attachment 380799 [details]
patch

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

> Source/JavaScriptCore/runtime/Structure.cpp:1134
> +            out.print(comma, entry.key, ":", static_cast<int>(entry.offset), "|", "getter=", entry.attributes & PropertyAttribute::Accessor);

Will revert
Comment 9 Saam Barati 2019-10-14 10:45:19 PDT
Created attachment 380894 [details]
patch for landing
Comment 10 WebKit Commit Bot 2019-10-14 11:08:11 PDT
Comment on attachment 380894 [details]
patch for landing

Rejecting attachment 380894 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 380894, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Tools/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/13129920
Comment 11 Saam Barati 2019-10-14 11:30:34 PDT
Created attachment 380902 [details]
patch for landing
Comment 12 WebKit Commit Bot 2019-10-14 12:11:57 PDT
Comment on attachment 380902 [details]
patch for landing

Clearing flags on attachment: 380902

Committed r251085: <https://trac.webkit.org/changeset/251085>
Comment 13 WebKit Commit Bot 2019-10-14 12:11:59 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Carlos Garcia Campos 2019-10-14 23:10:19 PDT
*** Bug 202594 has been marked as a duplicate of this bug. ***