|Product:||WebKit||Reporter:||Cameron Zwarich (cpst) <zwarich>|
|Version:||528+ (Nightly build)|
Description Cameron Zwarich (cpst) 2008-10-18 00:13:33 PDT
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.
Comment 1 Geoffrey Garen 2008-10-20 10:25:22 PDT
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.
Comment 2 Geoffrey Garen 2008-10-20 10:25:50 PDT
BTW, the same issue has cause a >10X regression on the empty function call benchmark's first run.
Comment 3 Gavin Barraclough 2008-10-20 13:24:43 PDT
(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.
Comment 4 Gavin Barraclough 2008-10-23 12:48:39 PDT
Created attachment 24609 [details] The patch Missing changelog, a wash on performance (other than in pathological case).
Comment 6 Oliver Hunt 2008-10-23 14:40:16 PDT
Comment on attachment 24619 [details] changelog++ r=me
Comment 7 Gavin Barraclough 2008-10-23 15:31:12 PDT