Closed
Bug 801775
Opened 12 years ago
Closed 12 years ago
IonMonkey: Filter logging spew based on script's origin
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: nbp, Assigned: nbp)
Details
(Whiteboard: [ion:t])
Attachments
(1 file)
5.24 KB,
patch
|
sstangl
:
review+
|
Details | Diff | Splinter Review |
Add an environment variable to enable C1/Ion spewer only when the filter match the origin of the script. This is useful to track reasons of bailouts in large scripts, such as v8, octane and maybe browsing sessions.
Such as Ion spew of bug 799818 can be filtered to produce less a few compilation results instead of hundreds:
IONFILTER=pdfjs.js:17068,pdfjs.js:26758 IONFLAGS=filter,logs ./dbg/js …
Attachment #671522 -
Flags: review?(sstangl)
![]() |
||
Updated•12 years ago
|
Whiteboard: [ion:t]
Comment 1•12 years ago
|
||
Comment on attachment 671522 [details] [diff] [review]
Filter logging spew based on script origin.
Review of attachment 671522 [details] [diff] [review]:
-----------------------------------------------------------------
Needs a comment explaining the format of IONFILTER, with some examples. Since this patch is debug-only, it is OK.
::: js/src/ion/IonSpewer.cpp
@@ +35,5 @@
> #undef IONSPEW_CHANNEL
> };
>
> +static bool
> +ContainsLocation(const char *filename, const size_t line = size_t(-1))
FilterContainsLocation()
@@ +45,5 @@
> + static const char *filter = getenv("IONFILTER");
> + static size_t filelen = strlen(filename);
> + const char *index = strstr(filter, filename);
> + while (index) {
> + if (index == filter || index[-1] == ',') {
This logic won't work for files with commas in their names, but I guess that's OK.
@@ +70,5 @@
> ion::IonSpewNewFunction(MIRGraph *graph, JSScript *function)
> {
> if (!js_IonOptions.parallelCompilation)
> + if (ContainsLocation(function->filename, function->lineno))
> + ionspewer.beginFunction(graph, function);
Outer if() needs braces, but it might looks nicer to just invert the condition on js_IonOptions.parallelCompilation and return early.
It might be better to hide the ContainsLocation() logic in beginFunction(), so that we don't accidentally call the latter.
@@ +122,5 @@
>
> +bool
> +IonSpewer::enabled()
> +{
> + return inited_ && graph && function;
Can graph be set but function unset?
@@ +269,5 @@
> EnableChannel(IonSpew_Pools);
> if (ContainsFlag(env, "logs"))
> EnableIonDebugLogging();
> + if (ContainsFlag(env, "filter"))
> + FilterLogs = true;
This seems unnecessary -- why not just detect whether the envvar IONFILTER is set, and use it if it is?
::: js/src/ion/IonSpewer.h
@@ +86,5 @@
> ~IonSpewer();
>
> bool init();
> void beginFunction(MIRGraph *graph, JSScript *);
> + bool enabled();
"enabled()" is not the right name for this function. "isSpewingFunction()"?
Attachment #671522 -
Flags: review?(sstangl) → review+
Comment 2•12 years ago
|
||
Why hasn't this patch get into the tree? Looks very handy!
Assignee | ||
Comment 3•12 years ago
|
||
I kept this patch on hold because it is not thread-safe as sstangl reported. I'll update it.
Assignee | ||
Comment 4•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9505ab7136b1
Here is the latest version of the patch, sadly, I just realized that I forgot to honor sstangl nit about adding a comment in the code to describe how to use it.
There is no threading issues since the modification of the flag is updated on the main thread before we start spewing anything.
The env variable spec:
without any IONFILTER environment variable, everything is spew as expected in debug builds. With IONFILTER set, all spew appear except for compilations of scripts which are not referred in IONFILTER. IONFILTER is extremelly convenient when combined with IONFLAGS=logs as it gives the ability to dump only the different compilation phase of only one function. This starts being usefull with real-life benchmarks as well as website with a debug build of Firefox.
![]() |
||
Comment 5•12 years ago
|
||
Assignee: general → nicolas.b.pierron
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in
before you can comment on or make changes to this bug.
Description
•