Open Bug 1330231 Opened 7 years ago Updated 2 years ago

DescribeScriptedCaller can be super inefficient on minified scripts with really long lines

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

People

(Reporter: ehsan.akhgari, Unassigned)

References

Details

(Keywords: triage-deferred)

See this profile: <https://clptr.io/2jC9bsJ>

We're spending ~15 *seconds* in PCToLineNumber, with SrcNoteLength and GetSrcNoteOffset showing up.  They both have a loop over the jssrcnote data structure, and my guess is that they're looping on something which is linear in terms of the number of characters on each line.

This is awful since these scripts are super common on the web, and we call this function from many places in Gecko.

Naveed, can you please find an owner?  Thanks!
Flags: needinfo?(nihsanullah)
See also bug 929950 and bug 1245213. Is this with the profiler enabled?

We should consider replacing the source note stuff for line/column with something we can binary search. It's been on my list but I've a lot of JIT perf work going on atm.

Jason, Shu: can we prioritize this?
See Also: → 1245213, 929950
(In reply to Jan de Mooij [:jandem] from comment #2)
> See also bug 929950 and bug 1245213. Is this with the profiler enabled?

Yes.

> We should consider replacing the source note stuff for line/column with
> something we can binary search. It's been on my list but I've a lot of JIT
> perf work going on atm.

That'd be wonderful!

I also had a silly idea.  Is it possible to annotate each bytecode with a lineno, making this O(1)?
(In reply to :Ehsan Akhgari (in Taipei, laggy response time) from comment #3)
> (In reply to Jan de Mooij [:jandem] from comment #2)
> > See also bug 929950 and bug 1245213. Is this with the profiler enabled?
> 
> Yes.
> 
> > We should consider replacing the source note stuff for line/column with
> > something we can binary search. It's been on my list but I've a lot of JIT
> > perf work going on atm.
> 
> That'd be wonderful!
> 
> I also had a silly idea.  Is it possible to annotate each bytecode with a
> lineno, making this O(1)?

That would use too much memory, I'm afraid. I agree with Jan that binary search is the right compromise here.

I've always hated source notes, I can put it on my list.
WFM, thanks for looking into this guys!
Any updates here?  DescribeScriptedCaller() is called in many places in Gecko, it would be nice if we didn't have this huge perf cliff...
See Also: → 1358709
Keywords: triage-deferred
Priority: -- → P3
Clearly ni on Naveed as he no longer works at Mozilla.
Flags: needinfo?(nihsanullah)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.