Bug 32561 - Provide a OProfile JIT agent (opagent) for the code generated by the JIT
: Provide a OProfile JIT agent (opagent) for the code generated by the JIT
Status: NEW
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore
: 528+ (Nightly build)
: PC Linux
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-12-15 08:28 PST by Holger Freyther
Modified: 2013-10-02 12:27 PDT (History)
14 users (show)

See Also:


Attachments
Proposal for implementing the feature (8.46 KB, patch)
2009-12-15 09:11 PST, Holger Freyther
no flags Details | Formatted Diff | Diff
Supports OProfile JIT agent (33.15 KB, patch)
2010-03-17 10:14 PDT, Gabor Loki
ggaren: review-
Details | Formatted Diff | Diff
sample report output (223.36 KB, text/plain)
2010-03-17 10:19 PDT, Gabor Loki
no flags Details
sample annotation (21.60 KB, text/plain)
2010-03-17 10:20 PDT, Gabor Loki
no flags Details
Supports OProfile JIT agent (43.02 KB, patch)
2010-04-16 08:06 PDT, Gabor Loki
no flags Details | Formatted Diff | Diff
Supports OProfile JIT agent (43.06 KB, patch)
2010-04-19 02:11 PDT, Gabor Loki
no flags Details | Formatted Diff | Diff
Supports OProfile JIT agent (43.55 KB, patch)
2010-04-20 01:56 PDT, Gabor Loki
no flags Details | Formatted Diff | Diff
Supports OProfile JIT agent (43.59 KB, patch)
2010-04-20 03:14 PDT, Gabor Loki
barraclough: review-
Details | Formatted Diff | Diff
jitoprofile.diff (41.52 KB, patch)
2010-12-07 01:35 PST, Xan Lopez
loki: review-
Details | Formatted Diff | Diff
jitprofile.diff (32.40 KB, patch)
2011-02-03 05:44 PST, Xan Lopez
no flags Details | Formatted Diff | Diff
jitprofile.diff (44.83 KB, patch)
2011-02-03 05:47 PST, Xan Lopez
no flags Details | Formatted Diff | Diff
jitprofile.diff (44.83 KB, patch)
2011-02-03 08:20 PST, Xan Lopez
no flags Details | Formatted Diff | Diff
jitprofile.diff (44.83 KB, patch)
2011-02-03 08:37 PST, Xan Lopez
no flags Details | Formatted Diff | Diff
jitprofile.diff (46.02 KB, patch)
2011-02-03 10:30 PST, Xan Lopez
no flags Details | Formatted Diff | Diff
jitprofile.diff (50.99 KB, patch)
2011-02-03 11:32 PST, Xan Lopez
oliver: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Holger Freyther 2009-12-15 08:28:09 PST
The interface described here http://oprofile.sourceforge.net/doc/devel/jit-api.html should be implemented. This will allow to see the function names in the system profile.
Comment 1 Holger Freyther 2009-12-15 09:11:46 PST
Created attachment 44882 [details]
Proposal for implementing the feature

This is the first compiling/linking version. At this point I would like to get feedback on:

    - The places I have patched. Are they the right one?
    - The way it is patched..
    - The buildsystem change
    - And if I should patch it in a similiar way for YARR (when JITTED)


What is not implemented:
   - Using the information in the generateJITCode methods to have a proper symbol name
Comment 2 Zoltan Herczeg 2009-12-15 09:56:30 PST
Hi,

wow, seems rather easy to work with oprofile.

>     - The places I have patched. Are they the right one?

I would consider to patch the platform dependent executableCopy(...) functions in JavaScriptCore/assembler directory, since all JIT code is allocated through this function. Although it does not provide information about the purpose of the JIT code. (whole function, common code (machine trampolines), property cache code fragment, etc.)

>     - The way it is patched..

Most of the code in JavaScriptCore refactored not to use plain void* pointers (Although I like void* pointers, since its purpose is clear). Perhaps passing a CodePtr (m_jitCode) would be the preferred way.

>     - The buildsystem change

I am not an expert on this topic, so it is fine by me.

>     - And if I should patch it in a similiar way for YARR (when JITTED)

Seems necessary if we don't want to change the executableCopy().
Comment 3 Gabor Loki 2010-03-17 10:14:48 PDT
Created attachment 50918 [details]
Supports OProfile JIT agent

This patch adds OProfile support for JSC.

Now the JS source/url path, function names and line information are available through OProfile's opreport and opannotate tools.
Comment 4 Gabor Loki 2010-03-17 10:19:04 PDT
Created attachment 50919 [details]
sample report output
Comment 5 Gabor Loki 2010-03-17 10:20:07 PDT
Created attachment 50920 [details]
sample annotation
Comment 6 Geoffrey Garen 2010-04-01 12:16:24 PDT
Comment on attachment 50918 [details]
Supports OProfile JIT agent

Looks like a really useful tool.

A few high-level comments:
* Can you only profile on Linux?

* Can we remove some of our other sampling mechanisms in favor of this?

* Is there a big advantage to making this a compile-time switch rather than a run-time switch?

* It would be nice if the interface to JITProfileAgent were a little less oprofile-specific, so it could be hooked up to other profiling back-ends without too much work.

It's not clear to me when you would enable OPROFILE_JIT_AGENT vs OPROFILE_JIT_AGENT_WITH_SAFE_UNLOAD. This is worth an explanation in a comment and in the ChangeLog.

+    void* vma()
+    {
+        return m_code;
+    }

You've duplicated the code() function. Also, the code function says, "Keep this private!" but you've made it public. That seems wrong.

You need to do something to ensure that no one can call this function while the LinkBuffer is in an invalid state.

+#if ENABLE(OPROFILE_JIT_AGENT)
+#define oprofile(a) a
+#else
+#define oprofile(a)
+#endif

+        oprofile(macro(op_oprofile, 3)) \

This is pretty clunky. Is there really such a big advantage to ensuring the op_oprofile opcode doesn't exist in non-oprofile builds? We don't go to the same lengths for things like op_debug.

+#if ENABLE(OPROFILE_JIT_AGENT)
+void JIT::emit_op_oprofile(Instruction* currentInstruction)
+{
+    m_lineInfos.append(LineInfoRecord(size(), currentInstruction[1].u.operand));
+}
+#endif

This function appears twice. It should appear only once.

Many instructions can span the same line. I think this function should coalesce adjacent instructions, to avoid constructing overly large data structures.

+    m_jitCode = JIT::compileAndAnnotate(scopeChainNode->globalData, codeBlock, symbolName);
+#else
     m_jitCode = JIT::compile(scopeChainNode->globalData, codeBlock);
+#endif

I don't think there should be two different ways to compile unless they actually compile differently. If we want compilation to annotate, but only in oprofile builds, the hooks for annotation should go into JIT::compile.

+        // TODO: make it thread safe

The way to make this thread-safe is to make it a property of the JSGlobalData, rather than a global.

+    typedef SegmentedVector<VirtualMemorySegment, 256> VirtualMemorySegmentVector;

Why does this need to be a SegmentedVector instead of just a Vector?

I think there enough to refine here to warrant an r-.
Comment 7 Gabor Loki 2010-04-02 06:11:08 PDT
> * Can you only profile on Linux?
> * Can we remove some of our other sampling mechanisms in favor of this?

I wish I could say yes to both questions, but OProfile is only for Linux. A kernel driver is needed at OS level. Probably it is not impossible to implement such a module on Mac as well ;)


> * Is there a big advantage to making this a compile-time switch rather than a
> run-time switch?

There were very few samples for OProfile part of WebKit and its agent as well. So, we can use run-time option, but it should be kept in mind not to grow and slow down JSC with useless checks where OProfile is not supported.

 
> It's not clear to me when you would enable OPROFILE_JIT_AGENT vs
> OPROFILE_JIT_AGENT_WITH_SAFE_UNLOAD. This is worth an explanation in a comment
> and in the ChangeLog.

SAFE_UNLOAD should be enabled by default. If it is disabled there could be some misplaced samples, but JSC runs faster.

> +    void* vma()
> +    {
> +        return m_code;
> +    }
> 
> You've duplicated the code() function. Also, the code function says, "Keep this private!" but you've made it public. That seems wrong.

Probably we can use executableAddress(), but the m_size has to be published anyway.

> You need to do something to ensure that no one can call this function while the LinkBuffer is in an invalid state.

What do you mean about invalid state? The state of JIT code (executable or flushed) does not matter for OProfile. Only the memory range is needed. The profiler will not touch that memory range at all.

Other things will be fixed. Thanks for your inputs!
Comment 8 Holger Freyther 2010-04-02 06:40:30 PDT
(In reply to comment #7)
> > * Can you only profile on Linux?
> > * Can we remove some of our other sampling mechanisms in favor of this?
> 
> I wish I could say yes to both questions, but OProfile is only for Linux. A
> kernel driver is needed at OS level. Probably it is not impossible to implement
> such a module on Mac as well ;)

The more natural choice on OS X would be to use DTrace but up to my knowledge the DTrace JIT support was never completed by SUN but we could look at the Java sourcecode to verify that.
Comment 9 Gabor Loki 2010-04-16 08:06:01 PDT
Created attachment 53530 [details]
Supports OProfile JIT agent

The patch is rewritten according to Geoffrey's comment, except one thing. It looks like that we cannot eliminate all JIT_PROFILE_AGENT macro, because there will be a small performance regression on ARM (~1-2%) if the agent is disabled.
Comment 10 Eric Seidel 2010-04-16 08:16:33 PDT
Attachment 53530 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/1626505
Comment 11 Gabor Loki 2010-04-16 12:58:18 PDT
> Build output: http://webkit-commit-queue.appspot.com/results/1626505

Oh, I missed that unused parameter. I will fix it. Apart from that, how does this patch look like?
Comment 12 Gabor Loki 2010-04-19 02:11:24 PDT
Created attachment 53661 [details]
Supports OProfile JIT agent

Unused parameter warning is fixed.
Comment 13 Eric Seidel 2010-04-19 02:19:04 PDT
Attachment 53661 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/1740058
Comment 14 Gabor Loki 2010-04-20 01:56:56 PDT
Created attachment 53784 [details]
Supports OProfile JIT agent

New files are added to Xcode project as well.
Comment 15 Eric Seidel 2010-04-20 02:17:02 PDT
Attachment 53784 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/1749023
Comment 16 Gabor Loki 2010-04-20 03:14:42 PDT
Created attachment 53793 [details]
Supports OProfile JIT agent

Err, it looks like that the Private flag was missing at JITProfileAgent.h.
Comment 17 Nikolas Zimmermann 2010-07-30 22:32:20 PDT
Can any of the JSC hackers have a look? This is pending since months!
Comment 18 Gavin Barraclough 2010-08-02 01:19:29 PDT
Comment on attachment 53793 [details]
Supports OProfile JIT agent

I think this needs tidying up a bit to reduce the amount to code added outside of the profiler.

Using op_profile_jit to gather the line number information should not be necessary; this should already be available from the codeblock from m_lineInfo.  If you can switch over to using the existing info it looks like the new op should be unnecessary.

YARR should not be calling into the JIT; these should be separate subsystems.  We should probably create a new top level directory (say, tools), and add this there.  (ooi ExecuableAllocator is also called from Yarr; this should not be in the jit directory).

The interpreter should not ASSERT_NOT_REACHED on the new op, if it remains – we now build both the interpreter and the JIT into the same build.

Adding work to non-profiling builds is a no-go; the changes in Executable need to be reverted.  The executable can be accessed from the codeblock, so the codeblock could just be passed in to the profiling agent, and the string could be calculated there.  The changes in JIT.h should be moved across into the agent; given the complexity of the JIT keeping the footprint of tools to the minimum will reduce complexity.  The JIT should only make a single function call here, passing the codeblock from which the data can be pulled.

The other changes throughout the JIT should be reduced to a single line macro call (i.e. compiles out to nothing if profiler is not built in, so skip having the extra lines for the compile guard).

Instead of passing strings to label the code, enum values should be used (different profiling tools may need different formats for names).

Enough comments for now, will be happy to rereview an updated patch.
Comment 19 Xan Lopez 2010-12-07 01:35:05 PST
Created attachment 75792 [details]
jitoprofile.diff

This patch tries to address all the review points raised in the last comment. I'm particularly unsure of whether the way I'm using the CodeBlock stuff to get the line info is correct :)
Comment 20 WebKit Review Bot 2010-12-07 08:36:24 PST
Attachment 75792 [details] did not pass style-queue:

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Early Warning System Bot 2010-12-07 09:16:11 PST
Attachment 75792 [details] did not build on qt:
Build output: http://queues.webkit.org/results/6780078
Comment 22 WebKit Review Bot 2010-12-07 09:37:35 PST
Attachment 75792 [details] did not pass style-queue:

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 23 WebKit Review Bot 2010-12-07 10:38:44 PST
Attachment 75792 [details] did not pass style-queue:

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 24 Build Bot 2010-12-07 10:48:07 PST
Attachment 75792 [details] did not build on win:
Build output: http://queues.webkit.org/results/6768083
Comment 25 WebKit Review Bot 2010-12-07 11:40:00 PST
Attachment 75792 [details] did not pass style-queue:

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 26 Eric Seidel 2010-12-07 12:42:44 PST
Attachment 75792 [details] did not build on mac:
Build output: http://queues.webkit.org/results/6736095
Comment 27 WebKit Review Bot 2010-12-07 21:31:41 PST
Attachment 75792 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2
Updating OpenSource
Incomplete data: Delta source ended unexpectedly at /usr/lib/git-core/git-svn line 5061

Died at WebKitTools/Scripts/update-webkit line 132.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 28 WebKit Review Bot 2010-12-10 20:46:37 PST
Attachment 75792 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/6950045
Comment 29 Xan Lopez 2011-01-25 15:38:37 PST
Any chance to have a look at the general direction of the code before this bitrots too much?
Comment 30 Gabor Loki 2011-02-01 01:15:25 PST
Comment on attachment 75792 [details]
jitoprofile.diff

First of all thanks for the update, I am deeply obliged to you!
Unfortunately this patch does not build on many platforms. We should work further on the patch to fix them. So, I am going to set the r? to r- because of the builds.

In order to fix the Qt build you should add
 * 'tools/JITProfileAgent.cpp \' line to the 'SOURCES +=' section in Source/JavaScriptCore/JavaScriptCore.pro
 * and '$$PWD/tools \' line to to the 'JAVASCRIPTCORE_INCLUDEPATH =' section in
Source/JavaScriptCore/JavaScriptCore.pri

Xan, could you update the patch again (because of the top Source directory) ?

I feel there is another issue about the codeBlock->lineInfo() function. Unfortunately I had no time to debug the code, but I have tried the jsc binary on ARM and the annotation was failed. There was no or not enough valid lineinfo data in the saved jitdump file. The correlation between source annotation and profile report was not sufficient. Xan, is the source annotation working for you?

One more thing. You changed the name of registerJITCodeImpl function to recordJITCodeImpl, but you forgot to change the dummy one (when JIT_OPROFILE_AGENT==0) to the similar name.
Comment 31 Xan Lopez 2011-02-01 09:57:50 PST
(In reply to comment #30)
> (From update of attachment 75792 [details])
> First of all thanks for the update, I am deeply obliged to you!
> Unfortunately this patch does not build on many platforms. We should work further on the patch to fix them. So, I am going to set the r? to r- because of the builds.

Hey, yeah, I was aware of this. I was basically interested in Gavin saying if my refactoring was along the lines of what he was asking for. In any case I'll try to get it building in all the EWS bots.

> 
> In order to fix the Qt build you should add
>  * 'tools/JITProfileAgent.cpp \' line to the 'SOURCES +=' section in Source/JavaScriptCore/JavaScriptCore.pro
>  * and '$$PWD/tools \' line to to the 'JAVASCRIPTCORE_INCLUDEPATH =' section in
> Source/JavaScriptCore/JavaScriptCore.pri
> 
> Xan, could you update the patch again (because of the top Source directory) ?

Yep, I've done this locally already, will upload the updated patch later today.

> 
> I feel there is another issue about the codeBlock->lineInfo() function. Unfortunately I had no time to debug the code, but I have tried the jsc binary on ARM and the annotation was failed. There was no or not enough valid lineinfo data in the saved jitdump file. The correlation between source annotation and profile report was not sufficient. Xan, is the source annotation working for you?

Yeah, it works as well as the original patch worked for me. Are you sure you are passing -p to the jsc binary?

> 
> One more thing. You changed the name of registerJITCodeImpl function to recordJITCodeImpl, but you forgot to change the dummy one (when JIT_OPROFILE_AGENT==0) to the similar name.

Yes, this is why it failed on GTK+ in EWS. I noticed it after uploading, it's also fixed locally. Thanks!
Comment 32 Xan Lopez 2011-02-03 05:44:44 PST
Created attachment 81054 [details]
jitprofile.diff

This should build at least in GTK+ and Qt.
Comment 33 WebKit Review Bot 2011-02-03 05:47:35 PST
Attachment 81054 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'GNUmakefile.am', u'Source/Ja..." exit_code: 1

Source/JavaScriptCore/jit/JITPropertyAccess.cpp:35:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 34 Xan Lopez 2011-02-03 05:47:55 PST
Created attachment 81055 [details]
jitprofile.diff

Better chances if I don't forget files.
Comment 35 WebKit Review Bot 2011-02-03 05:50:45 PST
Attachment 81055 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'GNUmakefile.am', u'Source/Ja..." exit_code: 1

Source/JavaScriptCore/jit/JITPropertyAccess.cpp:35:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 36 Collabora GTK+ EWS bot 2011-02-03 05:56:30 PST
Attachment 81054 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7689547
Comment 37 Early Warning System Bot 2011-02-03 06:10:23 PST
Attachment 81054 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7687873
Comment 38 Build Bot 2011-02-03 06:17:00 PST
Attachment 81055 [details] did not build on win:
Build output: http://queues.webkit.org/results/7691344
Comment 39 Build Bot 2011-02-03 06:24:33 PST
Attachment 81054 [details] did not build on win:
Build output: http://queues.webkit.org/results/7691346
Comment 40 Early Warning System Bot 2011-02-03 06:35:54 PST
Attachment 81055 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7690447
Comment 41 Gustavo Noronha (kov) 2011-02-03 08:03:34 PST
Attachment 81055 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7695173
Comment 42 Collabora GTK+ EWS bot 2011-02-03 08:18:54 PST
Attachment 81055 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7690472
Comment 43 Xan Lopez 2011-02-03 08:20:55 PST
Created attachment 81063 [details]
jitprofile.diff

This should fix GTK+ (different arch. than what I have at home). Not sure why Qt is not finding the header, can't find where I should add it.
Comment 44 WebKit Review Bot 2011-02-03 08:22:36 PST
Attachment 81063 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'GNUmakefile.am', u'Source/Ja..." exit_code: 1

Source/JavaScriptCore/jit/JITPropertyAccess.cpp:35:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 45 Gustavo Noronha (kov) 2011-02-03 08:30:33 PST
Attachment 81063 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7695179
Comment 46 Xan Lopez 2011-02-03 08:37:16 PST
Created attachment 81066 [details]
jitprofile.diff

C'mon.
Comment 47 WebKit Review Bot 2011-02-03 08:40:16 PST
Attachment 81066 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'GNUmakefile.am', u'Source/Ja..." exit_code: 1

Source/JavaScriptCore/jit/JITPropertyAccess.cpp:35:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 48 Early Warning System Bot 2011-02-03 08:48:27 PST
Attachment 81063 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7693335
Comment 49 Build Bot 2011-02-03 08:58:26 PST
Attachment 81063 [details] did not build on win:
Build output: http://queues.webkit.org/results/7690480
Comment 50 WebKit Review Bot 2011-02-03 08:59:06 PST
Attachment 81055 [details] did not build on mac:
Build output: http://queues.webkit.org/results/7691392
Comment 51 Build Bot 2011-02-03 09:16:52 PST
Attachment 81066 [details] did not build on win:
Build output: http://queues.webkit.org/results/7692316
Comment 52 Early Warning System Bot 2011-02-03 09:45:45 PST
Attachment 81066 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7694283
Comment 53 Gustavo Noronha (kov) 2011-02-03 10:20:39 PST
Attachment 81066 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7693349
Comment 54 Xan Lopez 2011-02-03 10:30:54 PST
Created attachment 81079 [details]
jitprofile.diff

Qt/WebKit2 duplicates the JSC includes, so I had to add the tools/ dir there too. Hopefully it will work now.
Comment 55 Build Bot 2011-02-03 11:24:39 PST
Attachment 81079 [details] did not build on win:
Build output: http://queues.webkit.org/results/7690507
Comment 56 Collabora GTK+ EWS bot 2011-02-03 11:29:01 PST
Attachment 81066 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7687929
Comment 57 Xan Lopez 2011-02-03 11:32:01 PST
Created attachment 81092 [details]
jitprofile.diff

This one has the XCode stuff, thanks to Holger.
Comment 58 Build Bot 2011-02-03 12:30:18 PST
Attachment 81092 [details] did not build on win:
Build output: http://queues.webkit.org/results/7698030
Comment 59 WebKit Review Bot 2011-02-03 15:53:49 PST
Attachment 81092 [details] did not build on mac:
Build output: http://queues.webkit.org/results/7691603
Comment 60 Oliver Hunt 2011-05-09 10:20:14 PDT
Hmmm, xan any progress on this?
Comment 61 Xan Lopez 2011-05-09 11:38:04 PDT
(In reply to comment #60)
> Hmmm, xan any progress on this?

Well, obviously I failed in making it build on Mac/Win, but I think in general the patch could still see some review from Gavin, since I tried to address all his concerns in the last round. It surely will need to be updated at this point, though, so feel free to r-.
Comment 62 Oliver Hunt 2011-05-09 12:01:00 PDT
Comment on attachment 81092 [details]
jitprofile.diff

Okay, i'll r- this.  I think we really just want this to be set up like other paltform specific things, eg. a header that defines a standard API then FooOProfile.cpp, FooSharkIFTheyEverDoSomethingToHelpUs.cpp, etc