Bug 20061 - Optimize array accesses with constant indices.
Summary: Optimize array accesses with constant indices.
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-16 14:10 PDT by Gavin Barraclough
Modified: 2010-03-02 17:29 PST (History)
0 users

See Also:


Attachments
Patch v1, adds op_get_by_index, optimizes array access with constant indices. (18.11 KB, patch)
2008-07-16 15:03 PDT, Gavin Barraclough
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2008-07-16 14:10:56 PDT
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).
Comment 1 Gavin Barraclough 2008-07-16 15:03:21 PDT
Created attachment 22319 [details]
Patch v1, adds op_get_by_index, optimizes array access with constant indices.
Comment 2 Oliver Hunt 2008-07-16 22:50:29 PDT
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.
Comment 3 Gavin Barraclough 2008-07-17 09:20:00 PDT
Updated and now no progression to minor degradation – will explore.
Comment 4 Geoffrey Garen 2008-07-18 15:41:14 PDT
Comment on attachment 22319 [details]
Patch v1, adds op_get_by_index, optimizes array access with constant indices.

Clearing review flag.
Comment 5 Gavin Barraclough 2008-07-28 15:58:11 PDT
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?
Comment 6 Gavin Barraclough 2010-03-02 17:29:25 PST
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.