Created attachment 62980 [details] Patch WeakGCPtr's should instead of directly pointing to the GC'd object should be directed to an array of pointers that can be updated for movable objects.
Created attachment 62986 [details] Patch Fixed merge conflicts.
Attachment 62986 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/wtf/BlockAllocator.cpp:21: You should add a blank line after implementation file's own header. [build/include_order] [4] JavaScriptCore/wtf/BlockAllocator.cpp:71: Use 0 instead of NULL. [readability/null] [5] JavaScriptCore/wtf/BlockAllocator.cpp:75: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] JavaScriptCore/runtime/GCHandle.h:39: This { should be at the end of the previous line [whitespace/braces] [4] JavaScriptCore/runtime/GCHandle.h:83: This { should be at the end of the previous line [whitespace/braces] [4] JavaScriptCore/wtf/wince/BlockAllocatorWinCE.cpp:21: You should add a blank line after implementation file's own header. [build/include_order] [4] JavaScriptCore/wtf/wince/BlockAllocatorWinCE.cpp:24: Alphabetical sorting problem. [build/include_order] [4] JavaScriptCore/wtf/wince/BlockAllocatorWinCE.cpp:25: Alphabetical sorting problem. [build/include_order] [4] JavaScriptCore/wtf/wince/BlockAllocatorWinCE.cpp:44: Use 0 instead of NULL. [readability/null] [5] JavaScriptCore/wtf/mac/BlockAllocatorMac.cpp:21: You should add a blank line after implementation file's own header. [build/include_order] [4] JavaScriptCore/wtf/mac/BlockAllocatorMac.cpp:24: Alphabetical sorting problem. [build/include_order] [4] JavaScriptCore/runtime/Collector.cpp:919: One line control clauses should not use braces. [whitespace/braces] [4] JavaScriptCore/runtime/WeakGCPtr.h:53: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] JavaScriptCore/runtime/WeakGCPtr.h:61: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] JavaScriptCore/runtime/GCHandle.cpp:27: You should add a blank line after implementation file's own header. [build/include_order] [4] JavaScriptCore/runtime/GCHandle.cpp:57: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] JavaScriptCore/wtf/win/BlockAllocatorWin.cpp:21: You should add a blank line after implementation file's own header. [build/include_order] [4] JavaScriptCore/wtf/win/BlockAllocatorWin.cpp:24: Alphabetical sorting problem. [build/include_order] [4] JavaScriptCore/wtf/win/BlockAllocatorWin.cpp:25: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 19 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 62986 [details] Patch The BlockAllocator stuff should be drop -- you should instead add the required APIs to the new PageAllocation class that gavin added a few days ago -- you may want to discuss details with him.
Created attachment 63491 [details] patch
Attachment 63491 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/runtime/GCHandle.h:39: This { should be at the end of the previous line [whitespace/braces] [4] JavaScriptCore/runtime/GCHandle.h:89: This { should be at the end of the previous line [whitespace/braces] [4] JavaScriptCore/runtime/WeakGCPtr.h:53: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] JavaScriptCore/runtime/WeakGCPtr.h:61: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] JavaScriptCore/runtime/GCHandle.cpp:56: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 5 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 63491 [details] did not build on qt: Build output: http://queues.webkit.org/results/3645322
Created attachment 63495 [details] patch
Created attachment 63509 [details] patch The previous build errors may be related to the lack of forwarding headers in the patch.
Attachment 63509 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 Total errors found: 0 in 0 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 63635 [details] patch (with patch!)
Comment on attachment 63635 [details] patch (with patch!) JavaScriptCore/runtime/Collector.cpp:938 + m_weakGCHandlePools.append(allocation); I think it would be better to use a smaller block allocator for WeakGCHandles, since there are usually so few of them. Maybe 32K instead of 256K? JavaScriptCore/runtime/GCHandle.h:66 + } The canonical names for these accessors in smart pointers are "get" and "set". JavaScriptCore/runtime/GCHandle.h:78 + } I would call these "nextInFreeList" and "setNextInFreeList". JavaScriptCore/runtime/GCHandle.cpp:74 + return 0; You can turn the "if" above into the ASSERT, and remove the ASSERT_NOT_REACHED and bogus return at the end.
Created attachment 63644 [details] patch
Created attachment 63646 [details] patch This should also limit the amount of memory allocated for the weakgchandlepools.
Comment on attachment 63646 [details] patch r=me
Comment on attachment 63646 [details] patch Clearing flags on attachment: 63646 Committed r64849: <http://trac.webkit.org/changeset/64849>
All reviewed patches have been landed. Closing bug.
This patch appears to break compile on Windows. Rolling out to verify.
Rollout was canceled. Thank you windows build fixer!