| Summary: | Fix up the VM sampling tool | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Geoffrey Garen <ggaren> | ||||
| Component: | JavaScriptCore | Assignee: | 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
Geoffrey Garen
2008-10-24 15:12:13 PDT
Created attachment 24656 [details]
patch
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. > 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! Committed revision 37891. |