Bug 197305

Summary: [Air] highOrderAdjacents in AbstractColoringAllocator::conservativeHeuristic should be some kind of array
Product: WebKit Reporter: Robin Morisset <rmorisset>
Component: JavaScriptCoreAssignee: Robin Morisset <rmorisset>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 154319    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
keith_miller: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews210 for win-future
none
Patch none

Robin Morisset
Reported 2019-04-25 19:47:29 PDT
Currently it is a HashSet, but it only ever holds at most registerCount() items. And linear search tends to be faster on such a small collection than hashing + searching in a HashSet. Further benefits include avoiding the allocation of the HashSet, not actually adding the nodes adjacent to V (since there are no duplicates in the adjacency lists).
Attachments
Patch (8.27 KB, patch)
2019-04-25 19:54 PDT, Robin Morisset 		no flags
DetailsFormatted DiffDiff
Patch (8.79 KB, patch)
2019-04-26 10:45 PDT, Robin Morisset 		no flags
DetailsFormatted DiffDiff
Patch (8.74 KB, patch)
2019-05-03 14:58 PDT, Robin Morisset 		keith_miller: review+
ews-watchlist: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ews210 for win-future (13.70 MB, application/zip)
2019-05-03 17:38 PDT, EWS Watchlist 		no flags
Details
Patch (8.83 KB, patch)
2019-09-09 13:15 PDT, Robin Morisset 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Robin Morisset
Comment 1 2019-04-25 19:54:37 PDT
Created attachment 368297 [details] Patch Approximately a 6% speedup for the register allocator.
EWS Watchlist
Comment 2 2019-04-25 19:55:50 PDT
Attachment 368297 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:234: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Robin Morisset
Comment 3 2019-04-26 10:45:23 PDT
Created attachment 368334 [details] Patch Fix style nits.
Robin Morisset
Comment 4 2019-05-03 14:58:38 PDT
Created attachment 368998 [details] Patch Just rebased.
EWS Watchlist
Comment 5 2019-05-03 17:38:35 PDT
Comment on attachment 368998 [details] Patch Attachment 368998 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12093904 New failing tests: http/tests/css/filters-on-iframes.html svg/dynamic-updates/SVGRadialGradientElement-svgdom-href-prop.html
EWS Watchlist
Comment 6 2019-05-03 17:38:37 PDT
Created attachment 369031 [details] Archive of layout-test-results from ews210 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews210 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Keith Miller
Comment 7 2019-09-09 11:30:53 PDT
Comment on attachment 368998 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368998&action=review r=me > Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:195 > + Vector<IndexType, 100> highOrderAdjacents; Why not just make this MacroAssembler::numGPRs + MacroAssembler::numFPRs? Alternatively you could make registerCount() constexpr.
Robin Morisset
Comment 8 2019-09-09 11:58:50 PDT
Comment on attachment 368998 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368998&action=review >> Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:195 >> + Vector<IndexType, 100> highOrderAdjacents; > > Why not just make this MacroAssembler::numGPRs + MacroAssembler::numFPRs? Alternatively you could make registerCount() constexpr. Making registerCount() constexpr is very tricky since it is defined as the length of a vector that is passed through quite a bit of code. I am currently testing MacroAssembler::numGPRs + MacroAssembler::numFPRs.
Robin Morisset
Comment 9 2019-09-09 13:15:37 PDT
Created attachment 378401 [details] Patch Submitting to EWS, I will cq+ it if it passes the tests.
WebKit Commit Bot
Comment 10 2019-09-09 17:23:56 PDT
Comment on attachment 378401 [details] Patch Clearing flags on attachment: 378401 Committed r249677: <https://trac.webkit.org/changeset/249677>
WebKit Commit Bot
Comment 11 2019-09-09 17:23:57 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 12 2019-09-09 17:24:28 PDT
<rdar://problem/55205490>
Note You need to log in before you can comment on or make changes to this bug.