Commit 22874998 authored by Z Nguyen-Huu's avatar Z Nguyen-Huu Committed by Commit Bot

Reland "Improve NumberToString when cache miss and Smi"

This is a reland of 1b35c0fa

Reason for revert: Seems to reliably break a numerics test:
https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20debug/31516

It was really slow and timeout with debug build run this test
mjsunit/math-exp-precision with --optimize-for-size because we resize
cache in CSA. Default this to runtime would avoid the timeout.

Also with --optimize-for-size, we don't have enough space to allocate
full-size cache so avoid to resize cache in this case.

In my local PC, time for this test decreases as follows.
Before: 52s
After: 3s

Original change's description:
> Improve NumberToString when cache miss and Smi
>
> Cache miss was handled in runtime before. This change add fast path for
> Smi in this case.
>
> Perf show 30% improvement for the following example.
> Before 67 ms
> After 42 ms
>
> const start = new Date();
> const MAX = 1000000;
> for (var i = 0; i < MAX; i++) {
>     i.toString();
> }
> const end = new Date();
> console.log("Time :"+ (end-start));
>
> Change-Id: I162e9c35f58551ca6a5a0efe79fb7c7b482a8594
> Bug: v8:10477
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2332866
> Commit-Queue: Z Nguyen-Huu <duongn@microsoft.com>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#69362}

Bug: v8:10477
Change-Id: I892a9007210032640d0bf22e61c8e7ad1a4377c4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2351398Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69413}
parent bd094d0d
......@@ -67,48 +67,23 @@ macro ToCharCode(input: int32): char8 {
%RawDownCast<char8>(input - 10 + kAsciiLowerCaseA);
}
// https://tc39.github.io/ecma262/#sec-number.prototype.tostring
transitioning javascript builtin NumberPrototypeToString(
js-implicit context: NativeContext, receiver: JSAny)(...arguments): String {
// 1. Let x be ? thisNumberValue(this value).
const x = ThisNumberValue(receiver, 'Number.prototype.toString');
// 2. If radix is not present, let radixNumber be 10.
// 3. Else if radix is undefined, let radixNumber be 10.
// 4. Else, let radixNumber be ? ToInteger(radix).
const radix: JSAny = arguments[0];
const radixNumber: Number = radix == Undefined ? 10 : ToInteger_Inline(radix);
// 5. If radixNumber < 2 or radixNumber > 36, throw a RangeError exception.
if (radixNumber < 2 || radixNumber > 36) {
ThrowRangeError(MessageTemplate::kToRadixFormatRange);
}
// 6. If radixNumber = 10, return ! ToString(x).
if (radixNumber == 10) {
return NumberToString(x);
}
// 7. Return the String representation of this Number
// value using the radix specified by radixNumber.
if (TaggedIsSmi(x)) {
@export
macro NumberToStringSmi(x: int32, radix: int32): String labels Slow {
const isNegative: bool = x < 0;
let n: int32 = Convert<int32>(UnsafeCast<Smi>(x));
let n: int32 = x;
if (!isNegative) {
// Fast case where the result is a one character string.
if (x < radixNumber) {
if (x < radix) {
return StringFromSingleCharCode(ToCharCode(n));
}
} else {
assert(isNegative);
if (n == kMinInt32) {
return runtime::DoubleToStringWithRadix(x, radixNumber);
goto Slow;
}
n = 0 - n;
}
const radix: int32 = Convert<int32>(radixNumber);
// Calculate length and pre-allocate the result string.
let temp: int32 = n;
let length: int32 = isNegative ? 1 : 0;
......@@ -117,8 +92,7 @@ transitioning javascript builtin NumberPrototypeToString(
length = length + 1;
}
assert(length > 0);
const strSeq = UnsafeCast<SeqOneByteString>(
AllocateSeqOneByteString(Unsigned(length)));
const strSeq: SeqOneByteString = AllocateSeqOneByteString(Unsigned(length));
let cursor: intptr = Convert<intptr>(length) - 1;
while (n > 0) {
const digit: int32 = n % radix;
......@@ -132,8 +106,44 @@ transitioning javascript builtin NumberPrototypeToString(
strSeq.chars[0] = 45;
} else {
assert(cursor == -1);
// In sync with Factory::SmiToString: If radix = 10 and positive number,
// update hash for string.
if (radix == 10) {
assert(strSeq.hash_field == kNameEmptyHashField);
strSeq.hash_field = MakeArrayIndexHash(Unsigned(x), Unsigned(length));
}
}
return strSeq;
}
// https://tc39.github.io/ecma262/#sec-number.prototype.tostring
transitioning javascript builtin NumberPrototypeToString(
js-implicit context: NativeContext, receiver: JSAny)(...arguments): String {
// 1. Let x be ? thisNumberValue(this value).
const x = ThisNumberValue(receiver, 'Number.prototype.toString');
// 2. If radix is not present, let radixNumber be 10.
// 3. Else if radix is undefined, let radixNumber be 10.
// 4. Else, let radixNumber be ? ToInteger(radix).
const radix: JSAny = arguments[0];
const radixNumber: Number = radix == Undefined ? 10 : ToInteger_Inline(radix);
// 5. If radixNumber < 2 or radixNumber > 36, throw a RangeError exception.
if (radixNumber < 2 || radixNumber > 36) {
ThrowRangeError(MessageTemplate::kToRadixFormatRange);
}
// 6. If radixNumber = 10, return ! ToString(x).
if (radixNumber == 10) {
return NumberToString(x);
}
// 7. Return the String representation of this Number
// value using the radix specified by radixNumber.
if (TaggedIsSmi(x)) {
return NumberToStringSmi(Convert<int32>(x), Convert<int32>(radixNumber))
otherwise return runtime::DoubleToStringWithRadix(x, radixNumber);
}
if (x == -0) {
......
......@@ -6711,12 +6711,40 @@ TNode<String> CodeStubAssembler::NumberToString(TNode<Number> input,
Signed(ChangeUint32ToWord(Int32Add(hash, hash)));
TNode<Object> smi_key =
UnsafeLoadFixedArrayElement(number_string_cache, entry_index);
GotoIf(TaggedNotEqual(smi_key, smi_input.value()), bailout);
Label if_smi_cache_missed(this);
GotoIf(TaggedNotEqual(smi_key, smi_input.value()), &if_smi_cache_missed);
// Smi match, return value from cache entry.
result = CAST(UnsafeLoadFixedArrayElement(number_string_cache, entry_index,
kTaggedSize));
Goto(&done);
BIND(&if_smi_cache_missed);
{
Label store_to_cache(this);
// Bailout when the cache is not full-size.
const int kFullCacheSize =
isolate()->heap()->MaxNumberToStringCacheSize();
Branch(IntPtrLessThan(number_string_cache_length,
IntPtrConstant(kFullCacheSize)),
bailout, &store_to_cache);
BIND(&store_to_cache);
{
// Generate string and update string hash field.
result = NumberToStringSmi(SmiToInt32(smi_input.value()),
Int32Constant(10), bailout);
// Store string into cache.
StoreFixedArrayElement(number_string_cache, entry_index,
smi_input.value());
StoreFixedArrayElement(number_string_cache,
IntPtrAdd(entry_index, IntPtrConstant(1)),
result.value());
Goto(&done);
}
}
}
BIND(&done);
return result.value();
......@@ -6726,6 +6754,8 @@ TNode<String> CodeStubAssembler::NumberToString(TNode<Number> input) {
TVARIABLE(String, result);
Label runtime(this, Label::kDeferred), done(this, &result);
GotoIfForceSlowPath(&runtime);
result = NumberToString(input, &runtime);
Goto(&done);
......
......@@ -2918,7 +2918,8 @@ V8_INLINE Handle<String> CharToString(Factory* factory, const char* string,
void Factory::NumberToStringCacheSet(Handle<Object> number, int hash,
Handle<String> js_string) {
if (!number_string_cache()->get(hash * 2).IsUndefined(isolate())) {
if (!number_string_cache()->get(hash * 2).IsUndefined(isolate()) &&
!FLAG_optimize_for_size) {
int full_size = isolate()->heap()->MaxNumberToStringCacheSize();
if (number_string_cache()->length() != full_size) {
Handle<FixedArray> new_cache =
......
......@@ -9,7 +9,7 @@ extern class Name extends PrimitiveHeapObject {
}
bitfield struct NameHash extends uint32 {
hash_not_commputed: bool: 1 bit;
hash_not_computed: bool: 1 bit;
is_not_integer_index_mask: bool: 1 bit;
array_index_value: uint32: 24 bit;
array_index_length: uint32: 6 bit;
......@@ -38,7 +38,7 @@ type PublicSymbol extends Symbol;
type PrivateSymbol extends Symbol;
const kNameEmptyHashField: NameHash = NameHash{
hash_not_commputed: true,
hash_not_computed: true,
is_not_integer_index_mask: true,
array_index_value: 0,
array_index_length: 0
......@@ -46,3 +46,44 @@ const kNameEmptyHashField: NameHash = NameHash{
const kMaxCachedArrayIndexLength: constexpr uint32
generates 'Name::kMaxCachedArrayIndexLength';
const kMaxArrayIndexSize: constexpr uint32
generates 'Name::kMaxArrayIndexSize';
const kNofHashBitFields: constexpr int31
generates 'Name::kNofHashBitFields';
const kArrayIndexValueBits: constexpr int31
generates 'Name::kArrayIndexValueBits';
const kDoesNotContainCachedArrayIndexMask: constexpr uint32
generates 'Name::kDoesNotContainCachedArrayIndexMask';
const kIsNotIntegerIndexMask: constexpr uint32
generates 'Name::kIsNotIntegerIndexMask';
macro ContainsCachedArrayIndex(hash: uint32): bool {
return (hash & kDoesNotContainCachedArrayIndexMask) == 0;
}
const kArrayIndexValueBitsShift: uint32 = kNofHashBitFields;
const kArrayIndexLengthBitsShift: uint32 =
kNofHashBitFields + kArrayIndexValueBits;
macro TenToThe(exponent: uint32): uint32 {
assert(exponent <= 9);
let answer: int32 = 1;
for (let i: int32 = 0; i < Signed(exponent); i++) {
answer = answer * 10;
}
return Unsigned(answer);
}
macro MakeArrayIndexHash(value: uint32, length: uint32): NameHash {
// This is in sync with StringHasher::MakeArrayIndexHash.
assert(length <= kMaxArrayIndexSize);
const one: uint32 = 1;
assert(TenToThe(kMaxCachedArrayIndexLength) < (one << kArrayIndexValueBits));
let hash: uint32 = value;
hash = (hash << kArrayIndexValueBitsShift) |
(length << kArrayIndexLengthBitsShift);
assert((hash & kIsNotIntegerIndexMask) == 0);
assert(
(length <= kMaxCachedArrayIndexLength) == ContainsCachedArrayIndex(hash));
return %RawDownCast<NameHash>(hash);
}
......@@ -76,9 +76,9 @@ extern class ThinString extends String {
type DirectString extends String;
@export
macro AllocateSeqOneByteString(length: uint32): String {
macro AllocateSeqOneByteString(length: uint32): SeqOneByteString {
assert(length <= kStringMaxLength);
if (length == 0) return kEmptyString;
if (length == 0) return UnsafeCast<SeqOneByteString>(kEmptyString);
return new SeqOneByteString{
map: kOneByteStringMap,
hash_field: kNameEmptyHashField,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment