1296876 - TraceLogger: write files to $TLDIR

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

My /tmp directory keeps getting cluttered with multiple batches of tracelogger outputs, and it's hard to keep straight which is which. It would be nice to be able to pick an output directory.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: TraceLogger: write files to $TLDIR

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: TraceLogger: write files to $TLDIR

Oops, one hunk was in the wrong patch.

Comment 3

[Hannes Verschore [:h4writer]]

Comment on attachment 8783213 [details] [diff] [review]
TraceLogger: write files to $TLDIR

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

Nice! Could you also document this on https://github.com/h4writer/tracelogger ?

::: js/src/vm/TraceLoggingGraph.cpp
@@ +54,5 @@
> +            p++;
> +            if (*p == 'u')
> +                len += sizeof("4294967295") - 1;
> +            else if (*p == 'd')
> +                len += sizeof("999") - 1;

Can we add asserts that check this? If possible that might be good to make sure %d doesn't get confused as being any int32 value.

Comment 4

[Steve Fink [:sfink] [:s:]]

(In reply to Hannes Verschore [:h4writer] from comment #3)
> Comment on attachment 8783213 [details] [diff] [review]
> TraceLogger: write files to $TLDIR
> 
> Review of attachment 8783213 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nice! Could you also document this on
> https://github.com/h4writer/tracelogger ?

Yeah, I'll send you a PR for this and for displaying the thread names.

> ::: js/src/vm/TraceLoggingGraph.cpp
> @@ +54,5 @@
> > +            p++;
> > +            if (*p == 'u')
> > +                len += sizeof("4294967295") - 1;
> > +            else if (*p == 'd')
> > +                len += sizeof("999") - 1;
> 
> Can we add asserts that check this? If possible that might be good to make
> sure %d doesn't get confused as being any int32 value.

I don't know how, because the actual values are passed through varargs. At least without reimplementing a printf format string parser.

Oh! I guess I could at least check whether the final vsnprintf'ed string ended up too long. "Guess what? A second ago, you just trashed some random memory!"

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3fe209e9ca79
TraceLogger: write files to $TLDIR, r=h4writer

Comment 6

[Hannes Verschore [:h4writer]]

Attachment: Few fixes

Some small fixes I didn't see while reviewing.

Comment 7

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8784411 [details] [diff] [review]
Few fixes

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

Yeah, I tried actually using it yesterday, and ended up with the same fixes. If it's ok, I'll just land my current patch, which already incorporates what you have here.

My TL-related patch stack is waiting on a review in bug 1296878, though.

Comment 8

[Hannes Verschore [:h4writer]]

(In reply to Steve Fink [:sfink] [:s:] from comment #7)
> Comment on attachment 8784411 [details] [diff] [review]
> Few fixes
> 
> Review of attachment 8784411 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Yeah, I tried actually using it yesterday, and ended up with the same fixes.
> If it's ok, I'll just land my current patch, which already incorporates what
> you have here.
> 
> My TL-related patch stack is waiting on a review in bug 1296878, though.

Sure

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c12dbf24cac
TraceLogger: write files to $TLDIR, r=h4writer

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1c12dbf24cac