Bug 86568 - Add instrumentation to Vector to track used capacity, and dump useful info
Summary: Add instrumentation to Vector to track used capacity, and dump useful info
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks: 86281
  Show dependency treegraph
 
Reported: 2012-05-15 21:18 PDT by Simon Fraser (smfr)
Modified: 2013-01-04 00:52 PST (History)
9 users (show)

See Also:


Attachments
Patch (22.95 KB, patch)
2012-05-15 21:25 PDT, Simon Fraser (smfr)
bfulgham: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2012-05-15 21:18:28 PDT
Add instrumentation to Vector to track used capacity, and dump useful info
Comment 1 Simon Fraser (smfr) 2012-05-15 21:25:50 PDT
Created attachment 142141 [details]
Patch
Comment 2 WebKit Review Bot 2012-05-15 21:28:04 PDT
Attachment 142141 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xc..." exit_code: 1
Source/WTF/wtf/Vector.h:43:  Code inside a namespace should not be indented.  [whitespace/indent] [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 3 Brent Fulgham 2012-11-25 20:38:27 PST
Comment on attachment 142141 [details]
Patch

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

The whole patch should probably have been landed long ago; it probably doesn't impact non-Mac platforms at all. We're there particular cases where these statistics were helpful in debugging a problem that you can mention?

R=me.

> Source/WTF/wtf/Vector.cpp:56
> +    pthread_once(&initializeOnceKey, initializeOnce);

Should we have a FIXME here for other platforms?

> Source/WTF/wtf/Vector.cpp:125
> +    VectorHash::iterator it = vectorData.find(vector);

It might be clearer to handle the fail/log case as an early return here.

> Source/WTF/wtf/Vector.cpp:142
> +    long long m_totalSizeBytes; 

Would it be better to use something like int64_t? Actually, does it make sense to allow these to be signed? Maybe uint64_t would be better?
Comment 4 Julien Chaffraix 2012-11-26 09:18:49 PST
Comment on attachment 142141 [details]
Patch

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

Simon, it seems that this code will be dead the moment it lands as it requires some modification of the main even loop that no port implements. Is there a plan to add this tracking to any platform or even better DRT?

> Source/WTF/wtf/Vector.cpp:42
> +#if OS(DARWIN)

It's wrong to use OS(DARWIN) here, you should use USE(PTHREADS)

> Source/WTF/wtf/Vector.cpp:88
> +    }

Regular WebKit style puts the functions first and the members last.
Comment 5 Simon Fraser (smfr) 2012-11-26 10:18:23 PST
Someone is welcome to take this patch and fix it up so that it works for all ports. I'm unlikely to do that any time soon.
Comment 6 Eric Seidel (no email) 2013-01-04 00:52:52 PST
Attachment 142141 [details] was posted by a committer and has review+, assigning to Simon Fraser for commit.