Bug 21873

Summary: Fix up the VM sampling tool
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: JavaScriptCoreAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, sam, zwarich
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
patch sam: review+

Geoffrey Garen
Reported 2008-10-24 15:12:13 PDT
Patch coming.
Attachments
patch (99.74 KB, patch)
2008-10-24 15:12 PDT, Geoffrey Garen 		sam: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Geoffrey Garen
Comment 1 2008-10-24 15:12:49 PDT
Created attachment 24656 [details] patch
Gavin Barraclough
Comment 2 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.
Geoffrey Garen
Comment 3 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!
Geoffrey Garen
Comment 4 2008-10-25 13:00:07 PDT
Committed revision 37891.
Note You need to log in before you can comment on or make changes to this bug.