Bug 20061 – Optimize array accesses with constant indices.

• Component: JavaScriptCore
• Status: RESOLVED WONTFIX

[Gavin Barraclough]

Emit op_get_by_index / op_put_by_index instead of op_get_by_val / op_put_by_val, to optimize '[' NumberNode ']' cases.  (1.5% progression on SunSpider, patch following).

[Gavin Barraclough]

Created attachment 22319 [details]
Patch v1, adds op_get_by_index, optimizes array access with constant indices.

[Oliver Hunt]

Comment on attachment 22319 [details]
Patch v1, adds op_get_by_index, optimizes array access with constant indices.

Minor style issues:
  the if in BracketAccessorNode::emitCode is a single line so should not have braces
  ditto for FunctionCallBracketNode::emitCode, ForInNode::emitCode
  The else blocks in PostIncBracketNode::emitCode are single line, so shouldn't have braces
 
Other than those this looks good. r=me.

[Gavin Barraclough]

Updated and now no progression to minor degradation – will explore.

[Geoffrey Garen]

Comment on attachment 22319 [details]
Patch v1, adds op_get_by_index, optimizes array access with constant indices.

Clearing review flag.

[Gavin Barraclough]

Performance numbers.

Sunspider pre-patch: ~1625ms
Patching all but nodes.cpp: ~1630ms (adding new opcodes to Machine seem to cause a performance hit – saw the same thing trying to add an op_sample. :-/ )
Patching all: ~1645ms

Puzzling.  From profiling the change seems to be operating as expected – best guess, this is handled well anyway with the fast case at the top of op_get_by_val, and the increased code footprint results in the degradation?

[Gavin Barraclough]

Closing this bug for now.

Having looked again, it may be of interest to some benchmarks (e.g. raytrace) to look at optimizing these gets/puts, but the patch here is so far out of date (pre JIT) it probably makes most sense to just close this bug & open a fresh one, if we come back to this at a later date.