Bug 98138

Summary: Provide memory instrumentation for HashCountedSet
Product: WebKit Reporter: Yury Semikhatsky <yurys>
Component: Web Inspector (Deprecated)Assignee: Yury Semikhatsky <yurys>
Status: RESOLVED FIXED    
Severity: Normal CC: alph, apavlov, benjamin, bweinstein, cmarcelo, japhet, joepeck, keishi, loislo, macpherson, menard, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch pfeldman: review+

Description Yury Semikhatsky 2012-10-02 03:33:54 PDT
We need to be able to estimate memory footprint of any HashCountedSet.
Comment 1 Yury Semikhatsky 2012-10-02 03:40:12 PDT
Created attachment 166655 [details]
Patch
Comment 2 Ilya Tikhonovsky 2012-10-02 09:27:40 PDT
Comment on attachment 166655 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=166655&action=review

> Source/WTF/wtf/MemoryInstrumentationSequence.h:58
> +    SequenceMemoryInstrumentationTraits<typename Conditional<IsConvertibleToInteger<ValueType>::value, int, ValueType>::Type>::reportMemoryUsage(begin, end, info);

I don't understand what is the benefit of this check (IsConvertibleToInteger)?
As I understand it doesn't matter that some type is convertible. We have to filter out POD types.
I'd say we have to use IsInteger here or even IsArithmatic which includes floats.
Comment 3 Yury Semikhatsky 2012-10-02 10:53:40 PDT
Comment on attachment 166655 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=166655&action=review

>> Source/WTF/wtf/MemoryInstrumentationSequence.h:58
>> +    SequenceMemoryInstrumentationTraits<typename Conditional<IsConvertibleToInteger<ValueType>::value, int, ValueType>::Type>::reportMemoryUsage(begin, end, info);
> 
> I don't understand what is the benefit of this check (IsConvertibleToInteger)?
> As I understand it doesn't matter that some type is convertible. We have to filter out POD types.
> I'd say we have to use IsInteger here or even IsArithmatic which includes floats.

As the comment says the check is added mainly because of  iterators over enum values. Like integers they cannot have methods and would cause compilation errors. Neither IsInterger nor IsArithmetic would would return true for enum so they are not an option.
Comment 4 Ilya Tikhonovsky 2012-10-02 11:44:30 PDT
lgtm
Comment 5 Yury Semikhatsky 2012-10-02 23:15:24 PDT
Committed r130253: <http://trac.webkit.org/changeset/130253>