901022 - write to MOZ_PROFILING_FILE when the tracer is stopped

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: write to MOZ_PROFILING_FILE when the tracer is stopped

The documentation in VisualEventTracer.h says something about this, but either
the documentation is wrong or the code used to do this once upon a time.

Regardless, having this option is helpful for tracing on non-desktop devices.

I'm not attached to the environment variable name; it might even be better to
make it something else, since we have an "official" profiler nowadays.  Maybe
MOZ_TRACE_FILE?

Comment 2

[Honza Bambas (:mayhemer)]

Comment on attachment 785082 [details] [diff] [review]
write to MOZ_PROFILING_FILE when the tracer is stopped

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

What do you intend to do with the log?  Currently the visualizer (the extension) doesn't support (up)load of a log to present it.  Do you plan to add that functionality somehow?  We could have |nsIVisualEventTracerLog nsIVisualEventTracer.loadSnaphot(filepath)|.  However, please keep in mind that the visualizer has probably absolutely no security checking of the input right now.

::: xpcom/base/VisualEventTracer.cpp
@@ +585,5 @@
> +VisualEventTracerLog::WriteToProfilingFile()
> +{
> +  const char* filename = PR_GetEnv("MOZ_PROFILING_FILE");
> +  if (!filename) {
> +    return NS_OK;

Hmm.. thinking of throwing here.  When user sets the env wrong (wrong name) we should indicate non-silently we didn't write anything.

According the name, I think MOZ_TRACE_FILE is OK.

@@ +598,5 @@
> +  nsCString json;
> +  GetJSONString(json);
> +
> +  int32_t bytesWritten = PR_Write(fd, json.get(), json.Length());
> +  PR_Close(fd);

Main thread I/O :D

No, I really wouldn't bother here.

Just a reminder this can be quite a big!

::: xpcom/base/nsIVisualEventTracer.idl
@@ +12,5 @@
>   * is a log of various events that are monitored by a custom code
>   * instrumentation around the mozilla code base.
>   */
>  
> +[builtinclass, scriptable, uuid(713ee3ca-95e0-4085-8616-f6d64a9508ad)]

Really needed?

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Honza Bambas (:mayhemer) from comment #2)
> What do you intend to do with the log?  Currently the visualizer (the
> extension) doesn't support (up)load of a log to present it.  Do you plan to
> add that functionality somehow?  We could have |nsIVisualEventTracerLog
> nsIVisualEventTracer.loadSnaphot(filepath)|.  However, please keep in mind
> that the visualizer has probably absolutely no security checking of the
> input right now.

My modified about:timeline (https://github.com/froydnj/about-timeline) supports loading files into the viewer.  It's very useful for figuring out what's happening on a not-so-capable device.

> ::: xpcom/base/VisualEventTracer.cpp
> @@ +585,5 @@
> > +VisualEventTracerLog::WriteToProfilingFile()
> > +{
> > +  const char* filename = PR_GetEnv("MOZ_PROFILING_FILE");
> > +  if (!filename) {
> > +    return NS_OK;
> 
> Hmm.. thinking of throwing here.  When user sets the env wrong (wrong name)
> we should indicate non-silently we didn't write anything.

OK, I guess if we're getting called from JS, we can do that.

> According the name, I think MOZ_TRACE_FILE is OK.

OK, will change.

> @@ +598,5 @@
> > +  nsCString json;
> > +  GetJSONString(json);
> > +
> > +  int32_t bytesWritten = PR_Write(fd, json.get(), json.Length());
> > +  PR_Close(fd);
> 
> Main thread I/O :D

:P

> ::: xpcom/base/nsIVisualEventTracer.idl
> @@ +12,5 @@
> >   * is a log of various events that are monitored by a custom code
> >   * instrumentation around the mozilla code base.
> >   */
> >  
> > +[builtinclass, scriptable, uuid(713ee3ca-95e0-4085-8616-f6d64a9508ad)]
> 
> Really needed?

Probably not entirely, but I don't think anybody is going to be implementing these in JS anytime soon.  I can take them out if you like.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5685f2befbf7