Bug 202827 – Canonicalize how we prepare the prototype chain for inline caching

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

...

[Saam Barati]

Created attachment 380714 [details]
WIP

it begins

[Radar WebKit Bug Importer]

<rdar://problem/56193919>

[Saam Barati]

Created attachment 380762 [details]
WIP

[Saam Barati]

Created attachment 380796 [details]
patch

[Saam Barati]

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.

[Saam Barati]

Created attachment 380799 [details]
patch

[Yusuke Suzuki]

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)`.

[Saam Barati]

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

[Saam Barati]

Created attachment 380894 [details]
patch for landing

[WebKit Commit Bot]

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

[Saam Barati]

Created attachment 380902 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 380902 [details]
patch for landing

Clearing flags on attachment: 380902

Committed r251085: <https://trac.webkit.org/changeset/251085>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Carlos Garcia Campos]

*** Bug 202594 has been marked as a duplicate of this bug. ***