Current implementation has overloads for HashSet. This prevents us from instrumenting complex cases like Vector<HashSet<...> >.
Created attachment 165077 [details] patch for try bots. HashSet + Vector.
Comment on attachment 165077 [details] patch for try bots. HashSet + Vector. Clearing r?. Please remove changes to vector.
Created attachment 165558 [details] Patch
Comment on attachment 165558 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165558&action=review > Source/WTF/wtf/MemoryInstrumentationHashSet.h:57 > +void reportMemoryUsage(const HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>* const& hashTable, MemoryObjectInfo* memoryObjectInfo) Please put HashTable memory instrumentation into a separate file as it is going to be reused by HashMap and LinkedHashSet instrumentations.
Comment on attachment 165558 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165558&action=review > Source/WebKit/chromium/tests/MemoryInstrumentationTest.cpp:446 > + VisitedObjects visitedObjects; Please add a test for HashTable instrumentation and a test for HashSet owned by value.
Created attachment 165587 [details] Patch
(In reply to comment #4) > (From update of attachment 165558 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=165558&action=review > > > Source/WTF/wtf/MemoryInstrumentationHashSet.h:57 > > +void reportMemoryUsage(const HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>* const& hashTable, MemoryObjectInfo* memoryObjectInfo) > > Please put HashTable memory instrumentation into a separate file as it is going to be reused by HashMap and LinkedHashSet instrumentations. done. > > Source/WebKit/chromium/tests/MemoryInstrumentationTest.cpp:446 > > + VisitedObjects visitedObjects; > > Please add a test for HashTable instrumentation and a test for HashSet owned by value. HashTable is a wtf internal class which is used by HashSet, HashMap, etc and is never used directly in WebCore. Also it knows nothing about the internal structure of the values inside it. Thus it is not necessary to test its instrumentation directly.
(In reply to comment #7) > > Please add a test for HashTable instrumentation and a test for HashSet owned by value. > > HashTable is a wtf internal class which is used by HashSet, HashMap, etc and is never used directly in WebCore. Also it knows nothing about the internal structure of the values inside it. Thus it is not necessary to test its instrumentation directly. What about the second part of the commentary?
(In reply to comment #8) > (In reply to comment #7) > > > Please add a test for HashTable instrumentation and a test for HashSet owned by value. > > > > HashTable is a wtf internal class which is used by HashSet, HashMap, etc and is never used directly in WebCore. Also it knows nothing about the internal structure of the values inside it. Thus it is not necessary to test its instrumentation directly. > > What about the second part of the commentary? addMember function has its own mechanic for pointer vs reference instrumentation. And it is covered by ptrVsRef test.
Comment on attachment 165587 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165587&action=review > Source/WTF/wtf/MemoryInstrumentation.h:193 > + template<typename I> void addMembers(I iterator, I end) Can you change this to addCollectionElements(const T& collection) so that we don't need to .begin() and .end() in all call sites?
Committed r129637: <http://trac.webkit.org/changeset/129637>
Reopening to attach new patch.
(In reply to comment #12) > Reopening to attach new patch. Sorry, wrong bug.