Using tooling from bug 186698, loading can and running "notifyutil -p com.apple.WebKit.dumpHashTableCapacity" shows: Wasted capacity: 179008 bytes (used 83136 of 262144 bytes, utilization: 31.71%) - 1 allocations 1 0x11606c2a5 WTF::HashTable<unsigned long, WTF::KeyValuePair<unsigned long, JSC::GCAwareJITStubRoutine*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long, JSC::GCAwareJITStubRoutine*> >, WTF::IntHash<unsigned long>, WTF::HashMap<unsigned long, JSC::GCAwareJITStubRoutine*, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<JSC::GCAwareJITStubRoutine*> >::KeyValuePairTraits, WTF::HashTraits<unsigned long> >::HashTable() 2 0x11606c285 WTF::HashMap<unsigned long, JSC::GCAwareJITStubRoutine*, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<JSC::GCAwareJITStubRoutine*> >::HashMap() 3 0x116062b75 WTF::HashMap<unsigned long, JSC::GCAwareJITStubRoutine*, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<JSC::GCAwareJITStubRoutine*> >::HashMap() 4 0x116062b49 JSC::JITStubRoutineSet::JITStubRoutineSet() 5 0x116062bb5 JSC::JITStubRoutineSet::JITStubRoutineSet() 6 0x116010051 JSC::Heap::Heap(JSC::VM*, JSC::HeapType) 7 0x1160126b3 JSC::Heap::Heap(JSC::VM*, JSC::HeapType) 8 0x1166b8e72 JSC::VM::VM(JSC::VM::VMType, JSC::HeapType)
<rdar://problem/41189137>
It's over 800KB on nytimes.com: Wasted capacity: 848672 bytes (used 199904 of 1048576 bytes, utilization: 19.06%) - 1 allocations 1 0x11606c2a5 WTF::HashTable<unsigned long, WTF::KeyValuePair<unsigned long, JSC::GCAwareJITStubRoutine*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long, JSC::GCAwareJITStubRoutine*> >, WTF::IntHash<unsigned long>, WTF::HashMap<unsigned long, JSC::GCAwareJITStubRoutine*, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<JSC::GCAwareJITStubRoutine*> >::KeyValuePairTraits, WTF::HashTraits<unsigned long> >::HashTable() 2 0x11606c285 WTF::HashMap<unsigned long, JSC::GCAwareJITStubRoutine*, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<JSC::GCAwareJITStubRoutine*> >::HashMap() 3 0x116062b75 WTF::HashMap<unsigned long, JSC::GCAwareJITStubRoutine*, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<JSC::GCAwareJITStubRoutine*> >::HashMap() 4 0x116062b49 JSC::JITStubRoutineSet::JITStubRoutineSet() 5 0x116062bb5 JSC::JITStubRoutineSet::JITStubRoutineSet() 6 0x116010051 JSC::Heap::Heap(JSC::VM*, JSC::HeapType) 7 0x1160126b3 JSC::Heap::Heap(JSC::VM*, JSC::HeapType) 8 0x1166b8e72 JSC::VM::VM(JSC::VM::VMType, JSC::HeapType)
In Gmail, it sometimes takes 2MB lol.
Created attachment 368393 [details] Patch
Attachment 368393 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:96: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 368393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368393&action=review > Source/JavaScriptCore/ChangeLog:17 > + performance bots say. If this becomes a problem, we can introduce some optimization. One thing I come up with is that introducing Eden/Old notion to GCAwareJITStubRoutine and only use Eden routines for Eden collection (as we do for LargeAllocation).
Comment on attachment 368393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368393&action=review Some drive by comments ... > Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:92 > + if (m_routines.isEmpty()) > return; > - > - iter->value->m_mayBeExecuting = true; > + if (m_startOfRange > address || address >= m_endOfRange) > + return; By doing the routine range check in the fast mark(), you can remove these checks here and ASSERT(!m_routines.isEmpty()) and ASSERT(isJITPC(bitwise_cast<void*>(address))) here instead. > Source/JavaScriptCore/heap/JITStubRoutineSet.h:55 > + if (!isJITPC(bitwise_cast<void*>(address))) Just check (m_startOfRange > address || address >= m_endOfRange) here instead. That range should strictly be a subset of the isJITPC() range. You can ASSERT(isJITPC(...)) in markSlow() as a sanity check.
Comment on attachment 368393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368393&action=review >> Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:92 >> + return; > > By doing the routine range check in the fast mark(), you can remove these checks here and ASSERT(!m_routines.isEmpty()) and ASSERT(isJITPC(bitwise_cast<void*>(address))) here instead. Right! Fixed, that's nice. >> Source/JavaScriptCore/heap/JITStubRoutineSet.h:55 >> + if (!isJITPC(bitwise_cast<void*>(address))) > > Just check (m_startOfRange > address || address >= m_endOfRange) here instead. That range should strictly be a subset of the isJITPC() range. > > You can ASSERT(isJITPC(...)) in markSlow() as a sanity check. Fixed.
Created attachment 368406 [details] Patch
Created attachment 368408 [details] Patch
Attachment 368408 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:94: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 368408 [details] Patch Attachment 368408 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12017615 Number of test failures exceeded the failure limit.
Created attachment 368410 [details] Archive of layout-test-results from ews115 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 368408 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368408&action=review > Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:136 > if (!routine->m_mayBeExecuting) It is not directly related to this change, but I'm not sure why this is correct. Why don't we need to mark cells when stub routine is not executed right now but it is live?
Created attachment 368411 [details] Patch
Attachment 368411 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:94: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 368393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368393&action=review > Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:61 > + routine, The trailing , is not needed here. > Source/JavaScriptCore/heap/JITStubRoutineSet.h:76 > + uintptr_t m_startOfRange { 0 }; > + uintptr_t m_endOfRange { 0 }; Wouldn't buy you much, but you could use WTF::Range<uintptr_t> here if you wanted.
Comment on attachment 368393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368393&action=review >> Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:61 >> + routine, > > The trailing , is not needed here. Dropped. >> Source/JavaScriptCore/heap/JITStubRoutineSet.h:76 >> + uintptr_t m_endOfRange { 0 }; > > Wouldn't buy you much, but you could use WTF::Range<uintptr_t> here if you wanted. Sounds fine! fixed.
Comment on attachment 368408 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368408&action=review >> Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:136 >> if (!routine->m_mayBeExecuting) > > It is not directly related to this change, but I'm not sure why this is correct. Why don't we need to mark cells when stub routine is not executed right now but it is live? It is correct as long as we maintain JITStubRoutines by visitWeak.
Created attachment 368420 [details] Patch
Attachment 368420 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:95: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 368421 [details] Patch
Attachment 368421 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:95: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 368422 [details] Patch
Attachment 368422 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:95: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 368422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368422&action=review > Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:99 > + routine.routine->m_mayBeExecuting = true; I think we should track this information only when, 1. stub requires marking 2. stub is jettisoned We could make the size of array small I think. I'll do that.
Comment on attachment 368422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368422&action=review >> Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:99 >> + routine.routine->m_mayBeExecuting = true; > > I think we should track this information only when, > > 1. stub requires marking > 2. stub is jettisoned > > We could make the size of array small I think. I'll do that. For now, I'll start with this simple patch.
Created attachment 368443 [details] Patch
Attachment 368443 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:95: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 368443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368443&action=review r=me > Source/JavaScriptCore/ChangeLog:8 > + Our current mechanism of JITStubRoutineSet is too memory consuming. Basically we have HashMap<uintptr_t, StubRoutine*> and register "is too memory consuming" => "consumes more memory than needed". > Source/JavaScriptCore/ChangeLog:10 > + In Gmail, we see ~2MB table size. "see" => "see a"
Comment on attachment 368443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368443&action=review Thanks! >> Source/JavaScriptCore/ChangeLog:8 >> + Our current mechanism of JITStubRoutineSet is too memory consuming. Basically we have HashMap<uintptr_t, StubRoutine*> and register > > "is too memory consuming" => "consumes more memory than needed". Fixed. >> Source/JavaScriptCore/ChangeLog:10 >> + In Gmail, we see ~2MB table size. > > "see" => "see a" Fixed.
Committed r244745: <https://trac.webkit.org/changeset/244745>