912302 - Slim down TraceLogging output by shortening entry keys and timestamps

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Till Schneidereit [:till]]

Attachment: Slim down TraceLogging output by shortening entry keys and timestamps

This patch shortens all names for the various entries to one or two letters.

Additionally, the TraceLogging constructor now takes a timestamp and substracts that from all later timestamps, shortening them quite a bit.

Corresponding changes to generate.py coming in a pull request.

Comment 2

[Till Schneidereit [:till]]

Oh, and I forgot to mention: this reduces log file size by about 25%.

Comment 3

[Hannes Verschore [:h4writer]]

Comment on attachment 799197 [details] [diff] [review]
Slim down TraceLogging output by shortening entry keys and timestamps

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

::: js/src/TraceLogging.cpp
@@ +80,5 @@
> +    "0,ps", // stop script parsing
> +    "1,pl", // start lazy parsing
> +    "0,pl", // stop lazy parsing
> +    "1,pf", // start Function parsing
> +    "0,pf", // stop Function parsing

lowercase function

@@ +83,5 @@
> +    "1,pf", // start Function parsing
> +    "0,pf", // stop Function parsing
> +    "e,i",  // engine interpreter
> +    "e,b",  // engine baseline
> +    "e,j"   // engine IonMonkey

lowercase ionmonkey

Why do interpreter and ionmonkey start both with an i. Maybe we should use "o"?
i = interpreter
b = baseline engine
o = optimizing engine (ionmonkey)

@@ +89,5 @@
>  TraceLogging* TraceLogging::_defaultLogger = NULL;
>  
>  TraceLogging::TraceLogging()
>    : loggingTime(0),
> +    startupTime(rdtsc()),

nice

Comment 4

[Till Schneidereit [:till]]

Comment on attachment 799197 [details] [diff] [review]
Slim down TraceLogging output by shortening entry keys and timestamps

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

::: js/src/TraceLogging.cpp
@@ +80,5 @@
> +    "0,ps", // stop script parsing
> +    "1,pl", // start lazy parsing
> +    "0,pl", // stop lazy parsing
> +    "1,pf", // start Function parsing
> +    "0,pf", // stop Function parsing

Isn't this for parsing triggered by using `new Function("bla")`? In that case, lowercase function would be somewhat misleading, I think.

@@ +83,5 @@
> +    "1,pf", // start Function parsing
> +    "0,pf", // stop Function parsing
> +    "e,i",  // engine interpreter
> +    "e,b",  // engine baseline
> +    "e,j"   // engine IonMonkey

It's actually a "j", not an "i", but I like "o" much, much better.

Comment 5

[Hannes Verschore [:h4writer]]

(In reply to Till Schneidereit [:till] from comment #4)
> Comment on attachment 799197 [details] [diff] [review]
> Slim down TraceLogging output by shortening entry keys and timestamps
> 
> Review of attachment 799197 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/TraceLogging.cpp
> @@ +80,5 @@
> > +    "0,ps", // stop script parsing
> > +    "1,pl", // start lazy parsing
> > +    "0,pl", // stop lazy parsing
> > +    "1,pf", // start Function parsing
> > +    "0,pf", // stop Function parsing
> 
> Isn't this for parsing triggered by using `new Function("bla")`? In that
> case, lowercase function would be somewhat misleading, I think.

Makes sense.

Comment 6

[Till Schneidereit [:till]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb1d06db46eb

Comment 7

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/bb1d06db46eb