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
: WebKit
JavaScriptCore
: 528+ (Nightly build)
: PC Linux
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-12-15 08:28 PST by
Modified: 2013-10-02 12:27 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 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 From 2009-12-15 09:11:46 PST -------
Created an attachment (id=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 From 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 From 2010-03-17 10:14:48 PST -------
Created an attachment (id=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 From 2010-03-17 10:19:04 PST -------
Created an attachment (id=50919) [details]
sample report output
------- Comment #5 From 2010-03-17 10:20:07 PST -------
Created an attachment (id=50920) [details]
sample annotation
------- Comment #6 From 2010-04-01 12:16:24 PST -------
(From update of attachment 50918 [details])
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 From 2010-04-02 06:11:08 PST -------
> * 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 From 2010-04-02 06:40:30 PST -------
(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 From 2010-04-16 08:06:01 PST -------
Created an attachment (id=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 From 2010-04-16 08:16:33 PST -------
Attachment 53530 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/1626505
------- Comment #11 From 2010-04-16 12:58:18 PST -------
> 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 From 2010-04-19 02:11:24 PST -------
Created an attachment (id=53661) [details]
Supports OProfile JIT agent

Unused parameter warning is fixed.
------- Comment #13 From 2010-04-19 02:19:04 PST -------
Attachment 53661 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/1740058
------- Comment #14 From 2010-04-20 01:56:56 PST -------
Created an attachment (id=53784) [details]
Supports OProfile JIT agent

New files are added to Xcode project as well.
------- Comment #15 From 2010-04-20 02:17:02 PST -------
Attachment 53784 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/1749023
------- Comment #16 From 2010-04-20 03:14:42 PST -------
Created an attachment (id=53793) [details]
Supports OProfile JIT agent

Err, it looks like that the Private flag was missing at JITProfileAgent.h.
------- Comment #17 From 2010-07-30 22:32:20 PST -------
Can any of the JSC hackers have a look? This is pending since months!
------- Comment #18 From 2010-08-02 01:19:29 PST -------
(From update of attachment 53793 [details])
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 From 2010-12-07 01:35:05 PST -------
Created an attachment (id=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 From 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 From 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 From 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 From 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 From 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 From 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 From 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 From 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 From 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 From 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 From 2011-02-01 01:15:25 PST -------
(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.

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 From 2011-02-01 09:57:50 PST -------
(In reply to comment #30)
> (From update of attachment 75792 [details] [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 From 2011-02-03 05:44:44 PST -------
Created an attachment (id=81054) [details]
jitprofile.diff

This should build at least in GTK+ and Qt.
------- Comment #33 From 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 From 2011-02-03 05:47:55 PST -------
Created an attachment (id=81055) [details]
jitprofile.diff

Better chances if I don't forget files.
------- Comment #35 From 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 From 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 From 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 From 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 From 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 From 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 From 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 From 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 From 2011-02-03 08:20:55 PST -------
Created an attachment (id=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 From 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 From 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 From 2011-02-03 08:37:16 PST -------
Created an attachment (id=81066) [details]
jitprofile.diff

C'mon.
------- Comment #47 From 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 From 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 From 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 From 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 From 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 From 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 From 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 From 2011-02-03 10:30:54 PST -------
Created an attachment (id=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 From 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 From 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 From 2011-02-03 11:32:01 PST -------
Created an attachment (id=81092) [details]
jitprofile.diff

This one has the XCode stuff, thanks to Holger.
------- Comment #58 From 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 From 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 From 2011-05-09 10:20:14 PST -------
Hmmm, xan any progress on this?
------- Comment #61 From 2011-05-09 11:38:04 PST -------
(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 From 2011-05-09 12:01:00 PST -------
(From update of attachment 81092 [details])
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