Bug 23390 - SamplingTool is broken.
Summary: SamplingTool is broken.
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: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-01-16 14:44 PST by Gavin Barraclough
Modified: 2009-01-16 16:16 PST (History)
0 users

See Also:


Attachments
The patch (10.36 KB, patch)
2009-01-16 14:45 PST, Gavin Barraclough
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2009-01-16 14:44:35 PST
Should probably fix that.
Comment 1 Gavin Barraclough 2009-01-16 14:45:29 PST
Created attachment 26808 [details]
The patch
Comment 2 Geoffrey Garen 2009-01-16 15:13:44 PST
Comment on attachment 26808 [details]
The patch

> +        void samplingToolTrackCodeBlock()
> +        {
> +#if ENABLE(CODEBLOCK_SAMPLING)
> +#if PLATFORM(X86_64)
> +            move(ImmPtr(m_interpreter->sampler()->codeBlockSlot()), X86::ecx);
> +            storePtr(ImmPtr(m_codeBlock), X86::ecx);
> +#else
> +            storePtr(ImmPtr(m_codeBlock), m_interpreter->sampler()->codeBlockSlot());
> +#endif
> +#endif
> +        }
> +
> +#if ENABLE(OPCODE_SAMPLING)
> +        void samplingToolTrackCodeBlock(Instruction* instruction, bool inHostFunction=false)
> +        {
> +#if PLATFORM(X86_64)
> +            move(ImmPtr(m_interpreter->sampler()->sampleSlot()), X86::ecx);
> +            storePtr(ImmPtr(m_interpreter->sampler()->encodeSample(instruction, inHostFunction)), X86::ecx);
> +#else
> +            storePtr(ImmPtr(m_interpreter->sampler()->encodeSample(instruction, inHostFunction)), m_interpreter->sampler()->sampleSlot());
> +#endif
> +        }
> +#else
> +        void samplingToolTrackCodeBlock(Instruction*, bool) {}
> +#endif

I think it's confusing to have two functions named "samplingToolTrackCodeBlock", when one records a CodeBlock and the other records an Instruction and some flags.

How about "sampleCodeBlock" and "sampleInstruction" instead?

(You might also want to pass the CodeBlock* to sampleCodeBlock. It's a bit subtle that the function automatically supplies the CodeBlock you're currently compiling. Its real purpose, I think, is just to abstract out 32bit vs 64bit.)

r=me
Comment 3 Gavin Barraclough 2009-01-16 16:16:19 PST
fixed in 39993