[webkit-reviews] review requested: [Bug 20175] Fixes for Windows and non-AllInOne file build with SamplingTool, plus review fixes. : [Attachment 22514] Alternative patch - single build version.

[bugzilla-daemon at webkit.org]

Gavin Barraclough <barraclough at apple.com> has asked  for review:
Bug 20175: Fixes for Windows and non-AllInOne file build with SamplingTool,
plus review fixes.
https://bugs.webkit.org/show_bug.cgi?id=20175

Attachment 22514: Alternative patch - single build version.
https://bugs.webkit.org/attachment.cgi?id=22514&action=edit

------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
Geoff,

As a Sunday afternoon project I've taken another swing at a version of the
sampler that does not require a separate build – I've not marked this as
obsoleting the previous patch in case you don't like this idea and would prefer
I stuck with the previous version, but I hope this does look good.

The patch makes the sampler always available without impacting on performance
when not in use.  Introducing a new bytecode op_sample opcode resulted in a
minor regression, so I've use a form of op_debug for the sampling hook.

The presence of the SamplingTool::dump code to print results degrades
performance (I'm guessing I'm getting unlucky, and the larger ro_data section
for the strings is shuffling the binary around – just including the array of
opcode names is enough to give a minor hit), so I've left this compiled out by
default on a switch ENABLE(SAMPLING_TOOL_DUMP).  With or without the dump code
there are two new methods available - SamplingTool::getScopeSampleRecords and
ScopeSampleRecord::getLineCounts - to extract results from the sampler.

By default the tool uses the op_debug/DoSample method to trigger sampling,
planting a hook at the beginning of every line of javascript source-code – but
an I've left a compile option ENABLE(SAMPLING_TOOL_ALWAYS) to allow the sampler
to operate as it currently does.  Enabling the switch results a lower cost
per-event, but it updates state every bytecode rather than for every line in
the JS sourcecode – and as a result the two mechanisms have about the same
overhead – setting ENABLE(SAMPLING_TOOL_ALWAYS) giving us a higher precision
mechanism at the cost of requiring a special build (and likely only useful to
us, since end users should not care which bytecode op a sample is falling on).

Hope this look good to you,
cheers, G.