Chromium Code Reviews
Help | Chromium Project | Sign in
(31)

Issue 598062: Probe the symbol table for two character strings in native code... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 2 months ago by Søren Thygesen Gjesse
Modified:
2 years, 11 months ago
CC:
v8-dev_googlegroups.com
Visibility:
Public.

Description

Probe the symbol table for two character strings in native code

All two character string results from adding two strings and from sub string used to be handled in the runtime system as a lookup in the symbol table was done before allocating a new string. The native code for string add and sub string now probes the symbol cache for two character strings to avoid the runtime call. If the result string is not found in the symbol table within a fixed number of probes a new string is just allocated. Newly allocated two character strings are not added to the symbol table immediately.

Committed: http://code.google.com/p/v8/source/detail?r=3842

Patch Set 1 #

Total comments: 16

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+292 lines, -9 lines) Lint Patch
M src/ia32/codegen-ia32.h View 1 1 chunk +25 lines, -0 lines 0 comments 0 errors Download
M src/ia32/codegen-ia32.cc View 1 4 chunks +246 lines, -9 lines 0 comments 0 errors Download
M src/ia32/macro-assembler-ia32.h View 1 chunk +7 lines, -0 lines 0 comments 0 errors Download
M src/ia32/macro-assembler-ia32.cc View 1 chunk +14 lines, -0 lines 0 comments 0 errors Download
Trybot results:
Commit:

Messages

Total messages: 3
Søren Thygesen Gjesse
4 years, 2 months ago #1
Mads Ager (chromium)
LGTM http://codereview.chromium.org/598062/diff/1/2 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/598062/diff/1/2#newcode9904 src/ia32/codegen-ia32.cc:9904: Label *not_found) { Label * -> Label* http://codereview.chromium.org/598062/diff/1/2#newcode9910 ...
4 years, 2 months ago #2
Søren Thygesen Gjesse
4 years, 2 months ago #3
http://codereview.chromium.org/598062/diff/1/2
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/598062/diff/1/2#newcode9904
src/ia32/codegen-ia32.cc:9904: Label *not_found) {
On 2010/02/12 10:24:05, Mads Ager wrote:
> Label * -> Label*
> 

Done.

http://codereview.chromium.org/598062/diff/1/2#newcode9910
src/ia32/codegen-ia32.cc:9910: Label not_aray_index;
On 2010/02/12 10:24:05, Mads Ager wrote:
> not_aray_index -> not_array_index

Done.

http://codereview.chromium.org/598062/diff/1/2#newcode9929
src/ia32/codegen-ia32.cc:9929: __ shl(c2, 8);
On 2010/02/12 10:24:05, Mads Ager wrote:
> kBitsInByte?

Done (kBitsPerByte).

http://codereview.chromium.org/598062/diff/1/2#newcode9952
src/ia32/codegen-ia32.cc:9952: // chars:        two character string (al first
char and ah second char)
On 2010/02/12 10:24:05, Mads Ager wrote:
> al and ah -> byte 0 and byte 1?  Nothing guarentees that the register will be
> eax, right?

Done.

http://codereview.chromium.org/598062/diff/1/2#newcode9985
src/ia32/codegen-ia32.cc:9985: // If length is not 2 the string is no a
candidate.
On 2010/02/12 10:24:05, Mads Ager wrote:
> no -> not

Done.

http://codereview.chromium.org/598062/diff/1/2#newcode10000
src/ia32/codegen-ia32.cc:10000: __ push(scratch);  // Save candidate.
On 2010/02/12 10:24:05, Mads Ager wrote:
> On the fast path we do push,pop,push,pop - can we make another entry label for
> pop_next_probe[i]?  That should be the right fall-through label, so I think it
> would be easy to avoid the extra push/pop?

Good point. Ended up pushing mask and used that as a temporary to keep the
candidate unchanged in a register.

http://codereview.chromium.org/598062/diff/1/3
File src/ia32/codegen-ia32.h (right):

http://codereview.chromium.org/598062/diff/1/3#newcode775
src/ia32/codegen-ia32.h:775: void
Generate2CharacterSymbolTableProbe(MacroAssembler* masm,
On 2010/02/12 10:24:05, Mads Ager wrote:
> 2 -> Two?

Done.

http://codereview.chromium.org/598062/diff/1/3#newcode781
src/ia32/codegen-ia32.h:781: Label *not_found);
On 2010/02/12 10:24:05, Mads Ager wrote:
> Label * -> Label*

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6