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

NEW
Unassigned

Status

()

Core
JavaScript Engine
P3
normal
a year ago
9 months ago

People

(Reporter: Ehsan, Unassigned, NeedInfo)

Tracking

({triage-deferred})

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

a year ago
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: → bug 1245213, bug 929950
(Reporter)

Comment 3

a year ago
(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)?

Comment 4

a year ago
(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.
(Reporter)

Comment 5

a year ago
WFM, thanks for looking into this guys!
(Reporter)

Comment 6

a year ago
Any updates here?  DescribeScriptedCaller() is called in many places in Gecko, it would be nice if we didn't have this huge perf cliff...
(Reporter)

Updated

a year ago
See Also: → bug 1358709
Keywords: triage-deferred
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.