Bug 130750

Summary:

Repatch should support setters and plant calls to them directly

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, buildbot, ggaren, mark.lam, mhahnenberg, msaboff, oliver, ossy, rniwa, sam

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

131336

Bug Blocks:

129588

Attachments:

Description	Flags
almost done	none
the patch	ggaren: review+, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2	none
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2	none
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion	none
patch for landing	none

Filip Pizlo

Reported 2014-03-25 15:46:09 PDT

Patch forthcoming.

Attachments
almost done (11.46 KB, patch) 2014-04-07 21:41 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (15.52 KB, patch) 2014-04-07 21:53 PDT, Filip Pizlo	ggaren: review+ buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (539.62 KB, application/zip) 2014-04-07 23:07 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (491.83 KB, application/zip) 2014-04-07 23:39 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (588.38 KB, application/zip) 2014-04-07 23:56 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (541.52 KB, application/zip) 2014-04-08 00:19 PDT, Build Bot	no flags	Details
patch for landing (16.56 KB, patch) 2014-04-08 11:46 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2014-04-07 21:41:42 PDT

Created attachment 228800 [details] almost done

Filip Pizlo

Comment 2 2014-04-07 21:53:30 PDT

Created attachment 228802 [details] the patch

Filip Pizlo

Comment 3 2014-04-07 22:00:15 PDT

This is a 12x speed-up on setter microbenchmarks.

Geoffrey Garen

Comment 4 2014-04-07 22:19:28 PDT

Comment on attachment 228802 [details] the patch r=me Can you beef up setter.js to force a cached setter to miss the cache due to a redefined setter?

Filip Pizlo

Comment 5 2014-04-07 22:21:54 PDT

(In reply to comment #4) > (From update of attachment 228802 [details]) > r=me > > Can you beef up setter.js to force a cached setter to miss the cache due to a redefined setter? Sure!

Build Bot

Comment 6 2014-04-07 23:07:48 PDT

Comment on attachment 228802 [details] the patch Attachment 228802 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5760488755429376 New failing tests: platform/mac/fast/scrolling/scroll-iframe-latched-mainframe.html platform/mac/fast/scrolling/scroll-select-latched-mainframe.html js/regress/setter.html platform/mac/fast/scrolling/scroll-div-latched-mainframe.html

Build Bot

Comment 7 2014-04-07 23:07:55 PDT

Created attachment 228808 [details] Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

Build Bot

Comment 8 2014-04-07 23:38:57 PDT

Comment on attachment 228802 [details] the patch Attachment 228802 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5040769945567232 New failing tests: js/regress/setter.html

Build Bot

Comment 9 2014-04-07 23:39:02 PDT

Created attachment 228809 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Build Bot

Comment 10 2014-04-07 23:56:04 PDT

Comment on attachment 228802 [details] the patch Attachment 228802 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6551012919738368 New failing tests: platform/mac/fast/scrolling/scroll-iframe-latched-mainframe.html platform/mac/fast/scrolling/scroll-select-latched-mainframe.html js/regress/setter.html platform/mac/fast/scrolling/scroll-div-latched-mainframe.html

Build Bot

Comment 11 2014-04-07 23:56:07 PDT

Created attachment 228810 [details] Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

Build Bot

Comment 12 2014-04-08 00:19:43 PDT

Comment on attachment 228802 [details] the patch Attachment 228802 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6604913685561344 New failing tests: js/regress/setter.html

Build Bot

Comment 13 2014-04-08 00:19:47 PDT

Created attachment 228813 [details] Archive of layout-test-results from webkit-ews-08 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-08 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Filip Pizlo

Comment 14 2014-04-08 11:46:58 PDT

Created attachment 228862 [details] patch for landing

Filip Pizlo

Comment 15 2014-04-08 12:40:40 PDT

Landed in http://trac.webkit.org/changeset/166945

Csaba Osztrogonác

Comment 16 2014-04-08 13:05:41 PDT

(In reply to comment #15) > Landed in http://trac.webkit.org/changeset/166945 It broke 18 tests on 32 bit: http://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/248 ** The following JSC stress test failures have been introduced: mozilla-tests.yaml/ecma/ExecutionContexts/10.1.6.js.mozilla-baseline mozilla-tests.yaml/ecma/ExecutionContexts/10.1.6.js.mozilla-dfg-eager-no-cjit-validate-phases mozilla-tests.yaml/ecma/Expressions/11.6.1-1.js.mozilla mozilla-tests.yaml/ecma/Expressions/11.6.1-1.js.mozilla-baseline mozilla-tests.yaml/ecma/Expressions/11.6.1-2.js.mozilla-baseline mozilla-tests.yaml/ecma/Expressions/11.6.2-1.js.mozilla-baseline mozilla-tests.yaml/ecma/Types/8.6.2.1-1.js.mozilla-baseline stress/setter.js.default stress/setter.js.no-llint stress/setter.js.always-trigger-copy-phase stress/setter.js.no-cjit-validate-phases regress/script-tests/assign-custom-setter-polymorphic.js.no-llint regress/script-tests/assign-custom-setter.js.no-llint regress/script-tests/assign-custom-setter.js.dfg-eager regress/script-tests/assign-custom-setter.js.dfg-eager-no-cjit-validate jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout-no-llint jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout-no-cjit Results for JSC stress tests: 18 failures found.

Filip Pizlo

Comment 17 2014-04-08 13:07:13 PDT

(In reply to comment #16) > (In reply to comment #15) > > Landed in http://trac.webkit.org/changeset/166945 > It broke 18 tests on 32 bit: > http://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/248 > > ** The following JSC stress test failures have been introduced: > mozilla-tests.yaml/ecma/ExecutionContexts/10.1.6.js.mozilla-baseline > mozilla-tests.yaml/ecma/ExecutionContexts/10.1.6.js.mozilla-dfg-eager-no-cjit-validate-phases > mozilla-tests.yaml/ecma/Expressions/11.6.1-1.js.mozilla > mozilla-tests.yaml/ecma/Expressions/11.6.1-1.js.mozilla-baseline > mozilla-tests.yaml/ecma/Expressions/11.6.1-2.js.mozilla-baseline > mozilla-tests.yaml/ecma/Expressions/11.6.2-1.js.mozilla-baseline > mozilla-tests.yaml/ecma/Types/8.6.2.1-1.js.mozilla-baseline > stress/setter.js.default > stress/setter.js.no-llint > stress/setter.js.always-trigger-copy-phase > stress/setter.js.no-cjit-validate-phases > regress/script-tests/assign-custom-setter-polymorphic.js.no-llint > regress/script-tests/assign-custom-setter.js.no-llint > regress/script-tests/assign-custom-setter.js.dfg-eager > regress/script-tests/assign-custom-setter.js.dfg-eager-no-cjit-validate > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout-no-llint > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout-no-cjit > > Results for JSC stress tests: > 18 failures found. I am looking.

Filip Pizlo

Comment 18 2014-04-08 13:17:09 PDT

(In reply to comment #17) > (In reply to comment #16) > > (In reply to comment #15) > > > Landed in http://trac.webkit.org/changeset/166945 > > It broke 18 tests on 32 bit: > > http://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/248 > > > > ** The following JSC stress test failures have been introduced: > > mozilla-tests.yaml/ecma/ExecutionContexts/10.1.6.js.mozilla-baseline > > mozilla-tests.yaml/ecma/ExecutionContexts/10.1.6.js.mozilla-dfg-eager-no-cjit-validate-phases > > mozilla-tests.yaml/ecma/Expressions/11.6.1-1.js.mozilla > > mozilla-tests.yaml/ecma/Expressions/11.6.1-1.js.mozilla-baseline > > mozilla-tests.yaml/ecma/Expressions/11.6.1-2.js.mozilla-baseline > > mozilla-tests.yaml/ecma/Expressions/11.6.2-1.js.mozilla-baseline > > mozilla-tests.yaml/ecma/Types/8.6.2.1-1.js.mozilla-baseline > > stress/setter.js.default > > stress/setter.js.no-llint > > stress/setter.js.always-trigger-copy-phase > > stress/setter.js.no-cjit-validate-phases > > regress/script-tests/assign-custom-setter-polymorphic.js.no-llint > > regress/script-tests/assign-custom-setter.js.no-llint > > regress/script-tests/assign-custom-setter.js.dfg-eager > > regress/script-tests/assign-custom-setter.js.dfg-eager-no-cjit-validate > > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout > > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout-no-llint > > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout-no-cjit > > > > Results for JSC stress tests: > > 18 failures found. > > I am looking. Does not repro in release for me. Trying debug now...

Filip Pizlo

Comment 19 2014-04-08 13:23:01 PDT

(In reply to comment #18) > (In reply to comment #17) > > (In reply to comment #16) > > > (In reply to comment #15) > > > > Landed in http://trac.webkit.org/changeset/166945 > > > It broke 18 tests on 32 bit: > > > http://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/248 > > > > > > ** The following JSC stress test failures have been introduced: > > > mozilla-tests.yaml/ecma/ExecutionContexts/10.1.6.js.mozilla-baseline > > > mozilla-tests.yaml/ecma/ExecutionContexts/10.1.6.js.mozilla-dfg-eager-no-cjit-validate-phases > > > mozilla-tests.yaml/ecma/Expressions/11.6.1-1.js.mozilla > > > mozilla-tests.yaml/ecma/Expressions/11.6.1-1.js.mozilla-baseline > > > mozilla-tests.yaml/ecma/Expressions/11.6.1-2.js.mozilla-baseline > > > mozilla-tests.yaml/ecma/Expressions/11.6.2-1.js.mozilla-baseline > > > mozilla-tests.yaml/ecma/Types/8.6.2.1-1.js.mozilla-baseline > > > stress/setter.js.default > > > stress/setter.js.no-llint > > > stress/setter.js.always-trigger-copy-phase > > > stress/setter.js.no-cjit-validate-phases > > > regress/script-tests/assign-custom-setter-polymorphic.js.no-llint > > > regress/script-tests/assign-custom-setter.js.no-llint > > > regress/script-tests/assign-custom-setter.js.dfg-eager > > > regress/script-tests/assign-custom-setter.js.dfg-eager-no-cjit-validate > > > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout > > > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout-no-llint > > > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout-no-cjit > > > > > > Results for JSC stress tests: > > > 18 failures found. > > > > I am looking. > > Does not repro in release for me. Trying debug now... Oh yuck!! That assertion probably affects 64-bit too. It's a dumb assertion, will land fix shortly...

Filip Pizlo

Comment 20 2014-04-08 13:26:21 PDT

(In reply to comment #19) > (In reply to comment #18) > > (In reply to comment #17) > > > (In reply to comment #16) > > > > (In reply to comment #15) > > > > > Landed in http://trac.webkit.org/changeset/166945 > > > > It broke 18 tests on 32 bit: > > > > http://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/248 > > > > > > > > ** The following JSC stress test failures have been introduced: > > > > mozilla-tests.yaml/ecma/ExecutionContexts/10.1.6.js.mozilla-baseline > > > > mozilla-tests.yaml/ecma/ExecutionContexts/10.1.6.js.mozilla-dfg-eager-no-cjit-validate-phases > > > > mozilla-tests.yaml/ecma/Expressions/11.6.1-1.js.mozilla > > > > mozilla-tests.yaml/ecma/Expressions/11.6.1-1.js.mozilla-baseline > > > > mozilla-tests.yaml/ecma/Expressions/11.6.1-2.js.mozilla-baseline > > > > mozilla-tests.yaml/ecma/Expressions/11.6.2-1.js.mozilla-baseline > > > > mozilla-tests.yaml/ecma/Types/8.6.2.1-1.js.mozilla-baseline > > > > stress/setter.js.default > > > > stress/setter.js.no-llint > > > > stress/setter.js.always-trigger-copy-phase > > > > stress/setter.js.no-cjit-validate-phases > > > > regress/script-tests/assign-custom-setter-polymorphic.js.no-llint > > > > regress/script-tests/assign-custom-setter.js.no-llint > > > > regress/script-tests/assign-custom-setter.js.dfg-eager > > > > regress/script-tests/assign-custom-setter.js.dfg-eager-no-cjit-validate > > > > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout > > > > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout-no-llint > > > > jsc-layout-tests.yaml/js/script-tests/for-in-cached.js.layout-no-cjit > > > > > > > > Results for JSC stress tests: > > > > 18 failures found. > > > > > > I am looking. > > > > Does not repro in release for me. Trying debug now... > > Oh yuck!! That assertion probably affects 64-bit too. It's a dumb assertion, will land fix shortly... Fixed in http://trac.webkit.org/changeset/166952

Note You need to log in before you can comment on or make changes to this bug.