REGRESSION: the JavaScriptCore test js1_5/Regress/regress-159334.js takes extremely long to finish in Debug builds

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Cameron Zwarich (cpst)]

A recent change has caused js1_5/Regress/regress-159334.js to take a very long time to run in debug builds, which makes the tests very inconvenient to run.

[Geoffrey Garen]

The problem is that CodeBlock::getStubInfo is N^2, and Gavin's recent change to cache function calls caused much heavier reliance on it.

We should:

1. Change getStubInfo to use a hashtable, so it's no longer N^2
2. Stop using the structureIDInstructions vector to hold data other than instructions whose StructureIDs need reference counting, so structureIDInstructions returns to making sense.

[Geoffrey Garen]

BTW, the same issue has cause a >10X regression on the empty function call benchmark's first run.

[Gavin Barraclough]

(In reply to comment #1)
> The problem is that CodeBlock::getStubInfo is N^2, and Gavin's recent change to
> cache function calls caused much heavier reliance on it.
> 
> We should:
> 
> 1. Change getStubInfo to use a hashtable, so it's no longer N^2

Since the data is inherently sorted, simply binary chopping may make sense here, in the case of the call no search is actually necessary, we can simply pass a pointer to the function.

> 2. Stop using the structureIDInstructions vector to hold data other than
> instructions whose StructureIDs need reference counting, so
> structureIDInstructions returns to making sense.

I'm not sure I a benefit in holding the data required for property access caching in the interpreter and property access caching in CTI code in separate data structures, and it will mean performing two allocations to do so.  I do think a sensible division will be to move the information on calls into a separate data structure, since then these can be searched separately.  As such, we may want to look at renaming structureIDInstructions, since really I think we want this to be specifically data required for property access instructions.

[Gavin Barraclough]

Created attachment 24609 [details]
The patch

Missing changelog, a wash on performance (other than in pathological case).

[Gavin Barraclough]

Created attachment 24619 [details]
changelog++

[Oliver Hunt]

Comment on attachment 24619 [details]
changelog++

r=me

[Gavin Barraclough]

Sending        JavaScriptCore/ChangeLog
Sending        JavaScriptCore/VM/CTI.cpp
Sending        JavaScriptCore/VM/CTI.h
Sending        JavaScriptCore/VM/CodeBlock.cpp
Sending        JavaScriptCore/VM/CodeBlock.h
Sending        JavaScriptCore/VM/CodeGenerator.cpp
Sending        JavaScriptCore/VM/Machine.cpp
Transmitting file data .......
Committed revision 37831.