Bug 197306 - [JSC] linkPolymorphicCall now does GC
Summary: [JSC] linkPolymorphicCall now does GC
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-25 22:22 PDT by Yusuke Suzuki
Modified: 2019-04-26 16:32 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.75 KB, patch)
2019-04-25 23:23 PDT, Yusuke Suzuki
sbarati: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2019-04-25 22:22:03 PDT
[JSC] linkPolymorphicCall now does GC
Comment 1 Yusuke Suzuki 2019-04-25 23:23:02 PDT
Created attachment 368305 [details]
Patch
Comment 2 Yusuke Suzuki 2019-04-25 23:24:14 PDT
<rdar://problem/50006719>
Comment 3 EWS Watchlist 2019-04-26 01:41:48 PDT
Comment on attachment 368305 [details]
Patch

Attachment 368305 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/12002939

New failing tests:
stress/proxy-set-prototype-of.js.dfg-eager
stress/super-property-access-tdz.js.ftl-eager
stress/const-semantics.js.ftl-eager
stress/string-to-string-error.js.dfg-eager-no-cjit-validate
stress/lexical-let-loop-semantics.js.dfg-eager
stress/proxy-get-prototype-of.js.ftl-eager
stress/finally-for-in.js.ftl-eager
stress/dfg-object-prototype-of.js.dfg-eager
stress/proxy-set-prototype-of.js.ftl-eager-no-cjit
stress/proxy-revoke.js.ftl-eager-no-cjit
stress/dataview-jit-bounds-checks.js.ftl-eager
stress/proxy-get-prototype-of.js.ftl-eager-no-cjit
stress/const-tdz.js.ftl-eager-no-cjit
stress/typedarray-access-neutered.js.dfg-eager-no-cjit-validate
stress/import-syntax.js.ftl-eager
stress/async-await-mozilla.js.dfg-eager-no-cjit-validate
stress/arrow-functions-as-default-parameter-values.js.ftl-eager
stress/lexical-let-tdz.js.dfg-eager-no-cjit-validate
stress/string-to-string-error.js.ftl-eager
stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-eager-no-cjit
stress/global-import-function-should-return-a-promise-when-clearing-exceptions.js.dfg-eager-no-cjit-validate
stress/eval-func-decl-block-with-var-sinthesize.js.ftl-eager
stress/super-property-access-tdz.js.ftl-eager-no-cjit
stress/inline-call-to-recursive-tail-call.js.dfg-eager-no-cjit-validate
stress/dfg-reflect-get-prototype-of.js.dfg-eager
stress/typedarray-access-neutered.js.ftl-eager-no-cjit
stress/typedarray-access-neutered.js.dfg-eager
stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-eager
stress/const-tdz.js.dfg-eager-no-cjit-validate
stress/string-to-string-error.js.ftl-eager-no-cjit
stress/typedarray-access-neutered.js.no-cjit-collect-continuously
stress/es6-default-parameters.js.dfg-eager
stress/const-tdz.js.ftl-eager
stress/string-value-of-error.js.dfg-eager-no-cjit-validate
stress/tail-call-recognize.js.ftl-eager-no-cjit
stress/const-loop-semantics.js.dfg-eager
stress/proxy-get-prototype-of.js.dfg-eager-no-cjit-validate
stress/tail-call-host-call-throw.js.ftl-eager
stress/dfg-object-proto-getter.js.dfg-eager
stress/dfg-object-proto-accessor.js.dfg-eager
stress/to-object-intrinsic.js.dfg-eager-no-cjit-validate
stress/global-import-function-should-return-a-promise-when-clearing-exceptions.js.ftl-eager-no-cjit
stress/string-value-of-error.js.ftl-eager
stress/async-iteration-yield-star.js.ftl-eager-no-cjit
stress/lexical-let-tdz.js.dfg-eager
stress/typedarray-access-neutered.js.ftl-eager
stress/lexical-let-tdz.js.ftl-eager
stress/const-semantics.js.dfg-eager
stress/const-tdz.js.dfg-eager
stress/arrow-functions-as-default-parameter-values.js.ftl-eager-no-cjit
stress/super-property-access-tdz.js.dfg-eager-no-cjit-validate
stress/proxy-revoke.js.dfg-eager-no-cjit-validate
stress/es6-default-parameters.js.ftl-eager
stress/string-to-string-error.js.dfg-eager
stress/dataview-typedarray-toindex.js.dfg-eager-no-cjit-validate
stress/tail-call-recognize.js.dfg-eager
stress/proxy-revoke.js.ftl-eager
stress/arrowfunction-lexical-bind-superproperty.js.dfg-eager
stress/arrowfunction-lexical-bind-this-8.js.dfg-eager
stress/string-value-of-error.js.ftl-eager-no-cjit
stress/lexical-let-tdz.js.ftl-eager-no-cjit
stress/string-value-of-error.js.dfg-eager
stress/eval-func-decl-block-with-var-sinthesize.js.ftl-eager-no-cjit
stress/es6-default-parameters.js.dfg-eager-no-cjit-validate
stress/super-property-access.js.dfg-eager
stress/constant-folding-osr-exit.js.ftl-eager
stress/super-property-access-tdz.js.dfg-eager
stress/proxy-get-prototype-of.js.dfg-eager
stress/arrowfunction-lexical-bind-superproperty.js.dfg-eager-no-cjit-validate
stress/object-create-define.js.ftl-eager-no-cjit
stress/arrowfunction-tdz-3.js.ftl-eager
stress/arrow-functions-as-default-parameter-values.js.dfg-eager
stress/async-await-mozilla.js.ftl-eager
stress/eval-func-decl-block-with-var-sinthesize.js.dfg-eager-no-cjit-validate
stress/tail-call-recognize.js.ftl-eager
stress/lexical-let-loop-semantics.js.ftl-eager-no-cjit
stress/to-object-intrinsic.js.dfg-eager
stress/array-species-create-should-handle-masquerader.js.dfg-eager
Comment 4 Yusuke Suzuki 2019-04-26 02:56:15 PDT
Comment on attachment 368305 [details]
Patch

Looking into the test failures.
Comment 5 Saam Barati 2019-04-26 08:16:40 PDT
Comment on attachment 368305 [details]
Patch

Nice find.
Comment 6 Saam Barati 2019-04-26 08:17:46 PDT
(In reply to Build Bot from comment #3)
> Comment on attachment 368305 [details]
> Patch
> 
> Attachment 368305 [details] did not pass jsc-ews (mac):
> Output: https://webkit-queues.webkit.org/results/12002939
> 
> New failing tests:
> stress/proxy-set-prototype-of.js.dfg-eager
> stress/super-property-access-tdz.js.ftl-eager
> stress/const-semantics.js.ftl-eager
> stress/string-to-string-error.js.dfg-eager-no-cjit-validate
> stress/lexical-let-loop-semantics.js.dfg-eager
> stress/proxy-get-prototype-of.js.ftl-eager
> stress/finally-for-in.js.ftl-eager
> stress/dfg-object-prototype-of.js.dfg-eager
> stress/proxy-set-prototype-of.js.ftl-eager-no-cjit
> stress/proxy-revoke.js.ftl-eager-no-cjit
> stress/dataview-jit-bounds-checks.js.ftl-eager
> stress/proxy-get-prototype-of.js.ftl-eager-no-cjit
> stress/const-tdz.js.ftl-eager-no-cjit
> stress/typedarray-access-neutered.js.dfg-eager-no-cjit-validate
> stress/import-syntax.js.ftl-eager
> stress/async-await-mozilla.js.dfg-eager-no-cjit-validate
> stress/arrow-functions-as-default-parameter-values.js.ftl-eager
> stress/lexical-let-tdz.js.dfg-eager-no-cjit-validate
> stress/string-to-string-error.js.ftl-eager
> stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-eager-no-cjit
> stress/global-import-function-should-return-a-promise-when-clearing-
> exceptions.js.dfg-eager-no-cjit-validate
> stress/eval-func-decl-block-with-var-sinthesize.js.ftl-eager
> stress/super-property-access-tdz.js.ftl-eager-no-cjit
> stress/inline-call-to-recursive-tail-call.js.dfg-eager-no-cjit-validate
> stress/dfg-reflect-get-prototype-of.js.dfg-eager
> stress/typedarray-access-neutered.js.ftl-eager-no-cjit
> stress/typedarray-access-neutered.js.dfg-eager
> stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-eager
> stress/const-tdz.js.dfg-eager-no-cjit-validate
> stress/string-to-string-error.js.ftl-eager-no-cjit
> stress/typedarray-access-neutered.js.no-cjit-collect-continuously
> stress/es6-default-parameters.js.dfg-eager
> stress/const-tdz.js.ftl-eager
> stress/string-value-of-error.js.dfg-eager-no-cjit-validate
> stress/tail-call-recognize.js.ftl-eager-no-cjit
> stress/const-loop-semantics.js.dfg-eager
> stress/proxy-get-prototype-of.js.dfg-eager-no-cjit-validate
> stress/tail-call-host-call-throw.js.ftl-eager
> stress/dfg-object-proto-getter.js.dfg-eager
> stress/dfg-object-proto-accessor.js.dfg-eager
> stress/to-object-intrinsic.js.dfg-eager-no-cjit-validate
> stress/global-import-function-should-return-a-promise-when-clearing-
> exceptions.js.ftl-eager-no-cjit
> stress/string-value-of-error.js.ftl-eager
> stress/async-iteration-yield-star.js.ftl-eager-no-cjit
> stress/lexical-let-tdz.js.dfg-eager
> stress/typedarray-access-neutered.js.ftl-eager
> stress/lexical-let-tdz.js.ftl-eager
> stress/const-semantics.js.dfg-eager
> stress/const-tdz.js.dfg-eager
> stress/arrow-functions-as-default-parameter-values.js.ftl-eager-no-cjit
> stress/super-property-access-tdz.js.dfg-eager-no-cjit-validate
> stress/proxy-revoke.js.dfg-eager-no-cjit-validate
> stress/es6-default-parameters.js.ftl-eager
> stress/string-to-string-error.js.dfg-eager
> stress/dataview-typedarray-toindex.js.dfg-eager-no-cjit-validate
> stress/tail-call-recognize.js.dfg-eager
> stress/proxy-revoke.js.ftl-eager
> stress/arrowfunction-lexical-bind-superproperty.js.dfg-eager
> stress/arrowfunction-lexical-bind-this-8.js.dfg-eager
> stress/string-value-of-error.js.ftl-eager-no-cjit
> stress/lexical-let-tdz.js.ftl-eager-no-cjit
> stress/string-value-of-error.js.dfg-eager
> stress/eval-func-decl-block-with-var-sinthesize.js.ftl-eager-no-cjit
> stress/es6-default-parameters.js.dfg-eager-no-cjit-validate
> stress/super-property-access.js.dfg-eager
> stress/constant-folding-osr-exit.js.ftl-eager
> stress/super-property-access-tdz.js.dfg-eager
> stress/proxy-get-prototype-of.js.dfg-eager
> stress/arrowfunction-lexical-bind-superproperty.js.dfg-eager-no-cjit-validate
> stress/object-create-define.js.ftl-eager-no-cjit
> stress/arrowfunction-tdz-3.js.ftl-eager
> stress/arrow-functions-as-default-parameter-values.js.dfg-eager
> stress/async-await-mozilla.js.ftl-eager
> stress/eval-func-decl-block-with-var-sinthesize.js.dfg-eager-no-cjit-validate
> stress/tail-call-recognize.js.ftl-eager
> stress/lexical-let-loop-semantics.js.ftl-eager-no-cjit
> stress/to-object-intrinsic.js.dfg-eager
> stress/array-species-create-should-handle-masquerader.js.dfg-eager

Are these real? Maybe we want DeferGCForAWhile?
Comment 7 Yusuke Suzuki 2019-04-26 11:31:04 PDT
(In reply to Saam Barati from comment #6)
> (In reply to Build Bot from comment #3)
> > Comment on attachment 368305 [details]
> > Patch
> > 
> > Attachment 368305 [details] did not pass jsc-ews (mac):
> > Output: https://webkit-queues.webkit.org/results/12002939
> > 
> > New failing tests:
> > stress/proxy-set-prototype-of.js.dfg-eager
> > stress/super-property-access-tdz.js.ftl-eager
> > stress/const-semantics.js.ftl-eager
> > stress/string-to-string-error.js.dfg-eager-no-cjit-validate
> > stress/lexical-let-loop-semantics.js.dfg-eager
> > stress/proxy-get-prototype-of.js.ftl-eager
> > stress/finally-for-in.js.ftl-eager
> > stress/dfg-object-prototype-of.js.dfg-eager
> > stress/proxy-set-prototype-of.js.ftl-eager-no-cjit
> > stress/proxy-revoke.js.ftl-eager-no-cjit
> > stress/dataview-jit-bounds-checks.js.ftl-eager
> > stress/proxy-get-prototype-of.js.ftl-eager-no-cjit
> > stress/const-tdz.js.ftl-eager-no-cjit
> > stress/typedarray-access-neutered.js.dfg-eager-no-cjit-validate
> > stress/import-syntax.js.ftl-eager
> > stress/async-await-mozilla.js.dfg-eager-no-cjit-validate
> > stress/arrow-functions-as-default-parameter-values.js.ftl-eager
> > stress/lexical-let-tdz.js.dfg-eager-no-cjit-validate
> > stress/string-to-string-error.js.ftl-eager
> > stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-eager-no-cjit
> > stress/global-import-function-should-return-a-promise-when-clearing-
> > exceptions.js.dfg-eager-no-cjit-validate
> > stress/eval-func-decl-block-with-var-sinthesize.js.ftl-eager
> > stress/super-property-access-tdz.js.ftl-eager-no-cjit
> > stress/inline-call-to-recursive-tail-call.js.dfg-eager-no-cjit-validate
> > stress/dfg-reflect-get-prototype-of.js.dfg-eager
> > stress/typedarray-access-neutered.js.ftl-eager-no-cjit
> > stress/typedarray-access-neutered.js.dfg-eager
> > stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-eager
> > stress/const-tdz.js.dfg-eager-no-cjit-validate
> > stress/string-to-string-error.js.ftl-eager-no-cjit
> > stress/typedarray-access-neutered.js.no-cjit-collect-continuously
> > stress/es6-default-parameters.js.dfg-eager
> > stress/const-tdz.js.ftl-eager
> > stress/string-value-of-error.js.dfg-eager-no-cjit-validate
> > stress/tail-call-recognize.js.ftl-eager-no-cjit
> > stress/const-loop-semantics.js.dfg-eager
> > stress/proxy-get-prototype-of.js.dfg-eager-no-cjit-validate
> > stress/tail-call-host-call-throw.js.ftl-eager
> > stress/dfg-object-proto-getter.js.dfg-eager
> > stress/dfg-object-proto-accessor.js.dfg-eager
> > stress/to-object-intrinsic.js.dfg-eager-no-cjit-validate
> > stress/global-import-function-should-return-a-promise-when-clearing-
> > exceptions.js.ftl-eager-no-cjit
> > stress/string-value-of-error.js.ftl-eager
> > stress/async-iteration-yield-star.js.ftl-eager-no-cjit
> > stress/lexical-let-tdz.js.dfg-eager
> > stress/typedarray-access-neutered.js.ftl-eager
> > stress/lexical-let-tdz.js.ftl-eager
> > stress/const-semantics.js.dfg-eager
> > stress/const-tdz.js.dfg-eager
> > stress/arrow-functions-as-default-parameter-values.js.ftl-eager-no-cjit
> > stress/super-property-access-tdz.js.dfg-eager-no-cjit-validate
> > stress/proxy-revoke.js.dfg-eager-no-cjit-validate
> > stress/es6-default-parameters.js.ftl-eager
> > stress/string-to-string-error.js.dfg-eager
> > stress/dataview-typedarray-toindex.js.dfg-eager-no-cjit-validate
> > stress/tail-call-recognize.js.dfg-eager
> > stress/proxy-revoke.js.ftl-eager
> > stress/arrowfunction-lexical-bind-superproperty.js.dfg-eager
> > stress/arrowfunction-lexical-bind-this-8.js.dfg-eager
> > stress/string-value-of-error.js.ftl-eager-no-cjit
> > stress/lexical-let-tdz.js.ftl-eager-no-cjit
> > stress/string-value-of-error.js.dfg-eager
> > stress/eval-func-decl-block-with-var-sinthesize.js.ftl-eager-no-cjit
> > stress/es6-default-parameters.js.dfg-eager-no-cjit-validate
> > stress/super-property-access.js.dfg-eager
> > stress/constant-folding-osr-exit.js.ftl-eager
> > stress/super-property-access-tdz.js.dfg-eager
> > stress/proxy-get-prototype-of.js.dfg-eager
> > stress/arrowfunction-lexical-bind-superproperty.js.dfg-eager-no-cjit-validate
> > stress/object-create-define.js.ftl-eager-no-cjit
> > stress/arrowfunction-tdz-3.js.ftl-eager
> > stress/arrow-functions-as-default-parameter-values.js.dfg-eager
> > stress/async-await-mozilla.js.ftl-eager
> > stress/eval-func-decl-block-with-var-sinthesize.js.dfg-eager-no-cjit-validate
> > stress/tail-call-recognize.js.ftl-eager
> > stress/lexical-let-loop-semantics.js.ftl-eager-no-cjit
> > stress/to-object-intrinsic.js.dfg-eager
> > stress/array-species-create-should-handle-masquerader.js.dfg-eager
> 
> Are these real? Maybe we want DeferGCForAWhile?

Yes, I tested it on my home machine yesterday and it was real.
It seems that JITCode is changing and operationOptimize see unexpected JITType for the CodeBlock (rough investigation I did yesterday).
And my impression is that we should use DeferGCForAWhile. After I figured out what is happening, and if it is fixed with DeferGCForAWhile, I'll land this with DeferGCForAWhile and comment about the behavior.
Comment 8 Yusuke Suzuki 2019-04-26 14:55:53 PDT
(In reply to Yusuke Suzuki from comment #7)
> (In reply to Saam Barati from comment #6) 
> > Are these real? Maybe we want DeferGCForAWhile?
> 
> Yes, I tested it on my home machine yesterday and it was real.
> It seems that JITCode is changing and operationOptimize see unexpected
> JITType for the CodeBlock (rough investigation I did yesterday).
> And my impression is that we should use DeferGCForAWhile. After I figured
> out what is happening, and if it is fixed with DeferGCForAWhile, I'll land
> this with DeferGCForAWhile and comment about the behavior.

When GC happens, it may jettison CodeBlock, and it makes operationLinkPolymorphicCall's SlowPathReturnType result broken. We should not perform GC in linkPolymorphicCall. So, DeferGCForAWhile is the correct fix.
Comment 9 Yusuke Suzuki 2019-04-26 16:32:51 PDT
Committed r244711: <https://trac.webkit.org/changeset/244711>