| Summary: | JSBoundFunction should lazily generate its name string | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||||||||||||||
| Component: | JavaScriptCore | Assignee: | Saam Barati <saam> | ||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||
| Severity: | Normal | CC: | barraclough, benjamin, buildbot, commit-queue, fpizlo, ggaren, gskachkov, keith_miller, mark.lam, msaboff, oliver, ossy, rniwa, ryanhaddad, sukolsak, ticaiolima, 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=160738 | ||||||||||||||||||||
| Bug Depends on: | 160737 | ||||||||||||||||||||
| Bug Blocks: | |||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
Saam Barati
2016-08-08 16:57:31 PDT
Created attachment 285659 [details]
patch
Comment on attachment 285659 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=285659&action=review r=me > Source/JavaScriptCore/runtime/JSBoundFunction.cpp:177 > - function->finishCreation(vm, executable, length, makeString("bound ", name)); > + function->finishCreation(vm, executable, length, String()); Why not remove the name string from JSBoundFunction::finishCreation() altogether? Have it pass String() to its base instead. Comment on attachment 285659 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=285659&action=review >> Source/JavaScriptCore/runtime/JSBoundFunction.cpp:177 >> + function->finishCreation(vm, executable, length, String()); > > Why not remove the name string from JSBoundFunction::finishCreation() altogether? Have it pass String() to its base instead. Will do. I was thinking that the finishCreation was calling JSFunction::finishCreation. I didn't realize it was calling JSBoundFunction::finishCreation. Created attachment 285662 [details]
patch for landing
Comment on attachment 285662 [details] patch for landing Attachment 285662 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1841552 New failing tests: js/regress/bound-function-construction-performance.html inspector/runtime/getProperties.html Created attachment 285667 [details]
Archive of layout-test-results from ews103 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 285662 [details] patch for landing Attachment 285662 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1841556 New failing tests: js/regress/bound-function-construction-performance.html inspector/runtime/getProperties.html Created attachment 285669 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 285662 [details] patch for landing Attachment 285662 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1841585 New failing tests: js/regress/bound-function-construction-performance.html Created attachment 285670 [details]
Archive of layout-test-results from ews121 for ios-simulator-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
Comment on attachment 285662 [details] patch for landing Attachment 285662 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1841563 New failing tests: js/regress/bound-function-construction-performance.html inspector/runtime/getProperties.html Created attachment 285671 [details]
Archive of layout-test-results from ews114 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 285677 [details]
patch for landing
Created attachment 285681 [details]
patch for landing
Comment on attachment 285681 [details] patch for landing Clearing flags on attachment: 285681 Committed r204321: <http://trac.webkit.org/changeset/204321> All reviewed patches have been landed. Closing bug. (In reply to comment #16) > Comment on attachment 285681 [details] > patch for landing > > Clearing flags on attachment: 285681 > > Committed r204321: <http://trac.webkit.org/changeset/204321> The new regress/script-tests/bound-function-construction-performance.js times out in debug mode on all bots, see build.webkit.org for details. (In reply to comment #18) > (In reply to comment #16) > > Comment on attachment 285681 [details] > > patch for landing > > > > Clearing flags on attachment: 285681 > > > > Committed r204321: <http://trac.webkit.org/changeset/204321> > > The new regress/script-tests/bound-function-construction-performance.js > times out in debug mode on all bots, see build.webkit.org for details. We should be skipping this test in Debug mode. Is this not happening? (In reply to comment #19) > (In reply to comment #18) > > (In reply to comment #16) > > > Comment on attachment 285681 [details] > > > patch for landing > > > > > > Clearing flags on attachment: 285681 > > > > > > Committed r204321: <http://trac.webkit.org/changeset/204321> > > > > The new regress/script-tests/bound-function-construction-performance.js > > times out in debug mode on all bots, see build.webkit.org for details. > > We should be skipping this test in Debug mode. > Is this not happening? Looks like Ossy is talking about the Debug JSC tests. We need to look into skipping it for the JSC tests too. (In reply to comment #20) > Looks like Ossy is talking about the Debug JSC tests. We need to look into > skipping it for the JSC tests too. How about adding "//@ slow!"? (In reply to comment #21) > (In reply to comment #20) > > Looks like Ossy is talking about the Debug JSC tests. We need to look into > > skipping it for the JSC tests too. > > How about adding "//@ slow!"? slow doesn't increase the timeout, only forces to run before all normal tests Re-opened since this is blocked by bug 160737 Also, this broke inspector/model/remote-object-get-properties.html, as order of properties changed:
DISPLAYABLE PROPERTIES:
- name
- length
- arguments
- caller
+ length
+ arguments
+ caller
+ name
__proto__
targetFunction
boundThis
boundArgs
ALL PROPERTIES:
- name
- length
- arguments
- caller
+ length
+ arguments
+ caller
+ name
toString
apply
call
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Fmodel%2Fremote-object-get-properties.html
Back to RESOLVED/FIXED, as we didn't end up rolling out the patch, as far as I can tell. |