Bug 21873 - Fix up the VM sampling tool
Summary: Fix up the VM sampling tool
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-24 15:12 PDT by Geoffrey Garen
Modified: 2008-10-25 13:00 PDT (History)
3 users (show)

See Also:


Attachments
patch (99.74 KB, patch)
2008-10-24 15:12 PDT, Geoffrey Garen
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2008-10-24 15:12:13 PDT
Patch coming.
Comment 1 Geoffrey Garen 2008-10-24 15:12:49 PDT
Created attachment 24656 [details]
patch
Comment 2 Gavin Barraclough 2008-10-24 18:04:17 PDT
This looks fine to me, but you'll need to find someone legit to agree – unofficial r+!  :o)

My only concerns would be in Platform.h – it looks like the patch attached would turn on sampling as default, though I'm assuming this is not your intention when you land it.  Also, the way you have structured the ifdefs it looks like ENABLE_OPCODE_SAMPLING can be defined twice – some compilers will warn on this, so I don't know if you want to restructure the ifdefs to avoid this.

G.

Comment 3 Geoffrey Garen 2008-10-25 12:12:36 PDT
> My only concerns would be in Platform.h – it looks like the patch attached
> would turn on sampling as default, though I'm assuming this is not your
> intention when you land it.  

Oops! Fixed.

> Also, the way you have structured the ifdefs it
> looks like ENABLE_OPCODE_SAMPLING can be defined twice – some compilers will
> warn on this, so I don't know if you want to restructure the ifdefs to avoid
> this.

Restructured.

Thanks!
Comment 4 Geoffrey Garen 2008-10-25 13:00:07 PDT
Committed revision 37891.