901768 - Instrument the frontend::Compile functions with the trace logger.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicolas B. Pierron [:nbp]]

Attachment: Instrument the frontend::Compile functions

This patch is made such as we have a common way to trace through functions all over the engine.  As a consequence of this patch, I already filed bug 901178 and fixed it, these results should appear on http://alasal.be/ionmonkey/ which would be interesting, especially to see how much of the interpreter belongs to the parser in octane-codeload.

Comment 1

[Hannes Verschore [:h4writer]]

Comment on attachment 786043 [details] [diff] [review]
Instrument the frontend::Compile functions

Review of attachment 786043 [details] [diff] [review]:
-----------------------------------------------------------------

- This parsing happens on the main thread (on the same thread as execution of JS), right?
I'm asking since the tracelogger can't handle logs from different threads. I'm gonna add that someday, when I start logging background thread compilation ;).
I'll do that possible after I finally can generate the logs automatic.

- I'll make sure I update the python script and add some nice colors for these ;) Thanks!

::: js/src/TraceLogging.h
@@ +10,5 @@
>  #include "jsscript.h"
>  
> +namespace JS {
> +
> +struct CompileOptions;

I think this is the same as lowercase JS. So you should be able to put this in there (so a little bit lower).

@@ +37,5 @@
>          INFO_ENGINE_BASELINE,
>          INFO_ENGINE_IONMONKEY,
> +        INFO_PARSER_COMPILE_SCRIPT,
> +        INFO_PARSER_COMPILE_LAZY,
> +        INFO_PARSER_COMPILE_FUNCTION,

I would change this to

PARSER_COMPILE_SCRIPT_START,
PARSER_COMPILE_SCRIPT_STOP,
PARSER_COMPILE_LAZY_START,
PARSER_COMPILE_LAZY_STOP,
PARSER_COMPILE_FUNCTION_START,
PARSER_COMPILE_FUNCTION_STOP

instead of the INFO ones, since then we need to keep twice as much in memory when using these PARSER_START / INFO_PARSER_* combo.

I added INFO_ENGINE / SCRIPT_START / SCRIPT_STOP only, because it was a pain in the ass to do it using ION_START/ION_STOP / BASELINE_START/BASELINE_STOP / INTERPRETER_START/INTERPRETER_STOP, since we don't have script info for every START to baseline/ionmonkey.

Comment 2

[Nicolas B. Pierron [:nbp]]

(In reply to Hannes Verschore [:h4writer] from comment #1)
> - This parsing happens on the main thread (on the same thread as execution
> of JS), right?

Only in the browser with async scripts, when this would land we should probably land a way to catch the thread id if this is not the main thread.  In the mean time we can just skip over anything which is done in other threads.

> I'm asking since the tracelogger can't handle logs from different threads.
> I'm gonna add that someday, when I start logging background thread
> compilation ;).

So this should be fine, as long as we do not enable it in the browser (which is what I will try to do without the async. parsing)

> - I'll make sure I update the python script and add some nice colors for
> these ;) Thanks!

Cool.  I will add a new patch with the renamed variable.

Comment 3

[Nicolas B. Pierron [:nbp]]

Attachment: tracelog-parser.patch

Applied nits.

Comment 4

[Nicolas B. Pierron [:nbp]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/557f6d8be7f6

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/557f6d8be7f6

Comment 6

[Hannes Verschore [:h4writer]]

Attachment: bugXXX-fix-parser

Small fix, because everything after a "," is 'discarded' as extra info. As a result all "parser" will be cling together instead of having different logs for parser_lazy/parser_script/...

Comment 7

[Nicolas B. Pierron [:nbp]]

Comment on attachment 788066 [details] [diff] [review]
bugXXX-fix-parser

Review of attachment 788066 [details] [diff] [review]:
-----------------------------------------------------------------

Hum … sounds like a limitation in a python script.

Comment 8

[Hannes Verschore [:h4writer]]

(In reply to Nicolas B. Pierron [:nbp] from comment #7)
> Comment on attachment 788066 [details] [diff] [review]
> bugXXX-fix-parser
> 
> Review of attachment 788066 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hum … sounds like a limitation in a python script.

Thanks. It's more about consistency. Currently I only place extra info after the , (like filename(), lineno combo). And I want to keep it that way. I could possible adjust the python script, but it would be hacky and different from normal behaviour ;)

Comment 9

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2904a317c74b

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2904a317c74b