Skip to content

Commit 0cdf92d

Browse files
akroshgakroshg
committed
[MERGE #1839 @akroshg] Removing few unwanted IsDetached check
Merge pull request #1839 from akroshg:ta3 IsDetached check is not required in the code path where we are a. constructing a typedarray itself. b. when source is typedarray. Fixed that. Few macro benchmarks (yperf) get good win due to this fix.
2 parents ec000d3 + d6ed628 commit 0cdf92d

File tree

2 files changed

+172
-8
lines changed

2 files changed

+172
-8
lines changed

‎lib/Runtime/Library/TypedArray.cpp‎

Lines changed: 125 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ namespace Js
560560
}
561561
else if (jsArraySource)
562562
{
563-
newArray->SetObject(jsArraySource, newArray->GetLength(), offset);
563+
newArray->SetObjectNoDetachCheck(jsArraySource, newArray->GetLength(), offset);
564564
OUTPUT_TRACE(TypedArrayPhase, _u("Created a TypedArray from a JavaScript array source\n"));
565565
}
566566

@@ -948,7 +948,7 @@ namespace Js
948948
{
949949
for (uint32 i = 0; i < sourceLength; i++)
950950
{
951-
DirectSetItem(offset + i, source->DirectGetItem(i));
951+
DirectSetItemNoDetachCheck(offset + i, source->DirectGetItemNoDetachCheck(i));
952952
}
953953
}
954954
else
@@ -965,22 +965,58 @@ namespace Js
965965
DirectSetItem(offset + i, tmpArray->DirectGetItem(i));
966966
}
967967
}
968+
if (source->IsDetachedBuffer() || this->IsDetachedBuffer())
969+
{
970+
Throw::FatalInternalError();
971+
}
968972
}
969973
}
970974

971-
// Getting length from the source object can detach the typedarray, and thus set it's length as 0,
972-
// this is observable because RangeError will be thrown instead of a TypeError further down the line
973-
void TypedArrayBase::SetObject(RecyclableObject* source, uint32 targetLength, uint32 offset)
975+
uint32 TypedArrayBase::GetSourceLength(RecyclableObject* arraySource, uint32 targetLength, uint32 offset)
974976
{
975977
ScriptContext* scriptContext = GetScriptContext();
976-
uint32 sourceLength = JavascriptConversion::ToUInt32(JavascriptOperators::OP_GetProperty(source, PropertyIds::length, scriptContext), scriptContext);
978+
uint32 sourceLength = JavascriptConversion::ToUInt32(JavascriptOperators::OP_GetProperty(arraySource, PropertyIds::length, scriptContext), scriptContext);
977979
uint32 totalLength;
978980
if (UInt32Math::Add(offset, sourceLength, &totalLength) ||
979981
(totalLength > targetLength))
980982
{
981983
JavascriptError::ThrowRangeError(
982-
GetScriptContext(), JSERR_InvalidTypedArrayLength);
984+
scriptContext, JSERR_InvalidTypedArrayLength);
983985
}
986+
return sourceLength;
987+
}
988+
989+
void TypedArrayBase::SetObjectNoDetachCheck(RecyclableObject* source, uint32 targetLength, uint32 offset)
990+
{
991+
ScriptContext* scriptContext = GetScriptContext();
992+
uint32 sourceLength = GetSourceLength(source, targetLength, offset);
993+
Assert(!this->IsDetachedBuffer());
994+
995+
Var itemValue;
996+
Var undefinedValue = scriptContext->GetLibrary()->GetUndefined();
997+
for (uint32 i = 0; i < sourceLength; i++)
998+
{
999+
if (!source->GetItem(source, i, &itemValue, scriptContext))
1000+
{
1001+
itemValue = undefinedValue;
1002+
}
1003+
DirectSetItemNoDetachCheck(offset + i, itemValue);
1004+
}
1005+
1006+
if (this->IsDetachedBuffer())
1007+
{
1008+
// We cannot be detached when we are creating the typed array itself. Terminate if that happens.
1009+
Throw::FatalInternalError();
1010+
}
1011+
}
1012+
1013+
// Getting length from the source object can detach the typedarray, and thus set it's length as 0,
1014+
// this is observable because RangeError will be thrown instead of a TypeError further down the line
1015+
void TypedArrayBase::SetObject(RecyclableObject* source, uint32 targetLength, uint32 offset)
1016+
{
1017+
ScriptContext* scriptContext = GetScriptContext();
1018+
uint32 sourceLength = GetSourceLength(source, targetLength, offset);
1019+
9841020
Var itemValue;
9851021
Var undefinedValue = scriptContext->GetLibrary()->GetUndefined();
9861022
for (uint32 i = 0; i < sourceLength; i ++)
@@ -2668,6 +2704,70 @@ namespace Js
26682704
return BaseTypedDirectGetItem(index);
26692705
}
26702706

2707+
#define DIRECT_SET_NO_DETACH_CHECK(TypedArrayName, convertFn) \
2708+
template<> \
2709+
inline BOOL TypedArrayName##::DirectSetItemNoDetachCheck(__in uint32 index, __in Var value) \
2710+
{ \
2711+
return BaseTypedDirectSetItemNoDetachCheck(index, value, convertFn); \
2712+
}
2713+
2714+
DIRECT_SET_NO_DETACH_CHECK(Int8Array, JavascriptConversion::ToInt8);
2715+
DIRECT_SET_NO_DETACH_CHECK(Int8VirtualArray, JavascriptConversion::ToInt8);
2716+
DIRECT_SET_NO_DETACH_CHECK(Uint8Array, JavascriptConversion::ToUInt8);
2717+
DIRECT_SET_NO_DETACH_CHECK(Uint8VirtualArray, JavascriptConversion::ToUInt8);
2718+
DIRECT_SET_NO_DETACH_CHECK(Int16Array, JavascriptConversion::ToInt16);
2719+
DIRECT_SET_NO_DETACH_CHECK(Int16VirtualArray, JavascriptConversion::ToInt16);
2720+
DIRECT_SET_NO_DETACH_CHECK(Uint16Array, JavascriptConversion::ToUInt16);
2721+
DIRECT_SET_NO_DETACH_CHECK(Uint16VirtualArray, JavascriptConversion::ToUInt16);
2722+
DIRECT_SET_NO_DETACH_CHECK(Int32Array, JavascriptConversion::ToInt32);
2723+
DIRECT_SET_NO_DETACH_CHECK(Int32VirtualArray, JavascriptConversion::ToInt32);
2724+
DIRECT_SET_NO_DETACH_CHECK(Uint32Array, JavascriptConversion::ToUInt32);
2725+
DIRECT_SET_NO_DETACH_CHECK(Uint32VirtualArray, JavascriptConversion::ToUInt32);
2726+
DIRECT_SET_NO_DETACH_CHECK(Float32Array, JavascriptConversion::ToFloat);
2727+
DIRECT_SET_NO_DETACH_CHECK(Float32VirtualArray, JavascriptConversion::ToFloat);
2728+
DIRECT_SET_NO_DETACH_CHECK(Float64Array, JavascriptConversion::ToNumber);
2729+
DIRECT_SET_NO_DETACH_CHECK(Float64VirtualArray, JavascriptConversion::ToNumber);
2730+
DIRECT_SET_NO_DETACH_CHECK(Int64Array, JavascriptConversion::ToInt64);
2731+
DIRECT_SET_NO_DETACH_CHECK(Uint64Array, JavascriptConversion::ToUInt64);
2732+
DIRECT_SET_NO_DETACH_CHECK(Uint8ClampedArray, JavascriptConversion::ToUInt8Clamped);
2733+
DIRECT_SET_NO_DETACH_CHECK(Uint8ClampedVirtualArray, JavascriptConversion::ToUInt8Clamped);
2734+
DIRECT_SET_NO_DETACH_CHECK(BoolArray, JavascriptConversion::ToBool);
2735+
2736+
#define DIRECT_GET_NO_DETACH_CHECK(TypedArrayName) \
2737+
template<> \
2738+
inline Var TypedArrayName##::DirectGetItemNoDetachCheck(__in uint32 index) \
2739+
{ \
2740+
return BaseTypedDirectGetItemNoDetachCheck(index); \
2741+
}
2742+
2743+
#define DIRECT_GET_VAR_CHECK_NO_DETACH_CHECK(TypedArrayName) \
2744+
template<> \
2745+
inline Var TypedArrayName##::DirectGetItemNoDetachCheck(__in uint32 index) \
2746+
{ \
2747+
return DirectGetItemVarCheckNoDetachCheck(index); \
2748+
}
2749+
2750+
DIRECT_GET_NO_DETACH_CHECK(Int8Array);
2751+
DIRECT_GET_NO_DETACH_CHECK(Int8VirtualArray);
2752+
DIRECT_GET_NO_DETACH_CHECK(Uint8Array);
2753+
DIRECT_GET_NO_DETACH_CHECK(Uint8VirtualArray);
2754+
DIRECT_GET_NO_DETACH_CHECK(Int16Array);
2755+
DIRECT_GET_NO_DETACH_CHECK(Int16VirtualArray);
2756+
DIRECT_GET_NO_DETACH_CHECK(Uint16Array);
2757+
DIRECT_GET_NO_DETACH_CHECK(Uint16VirtualArray);
2758+
DIRECT_GET_NO_DETACH_CHECK(Int32Array);
2759+
DIRECT_GET_NO_DETACH_CHECK(Int32VirtualArray);
2760+
DIRECT_GET_NO_DETACH_CHECK(Uint32Array);
2761+
DIRECT_GET_NO_DETACH_CHECK(Uint32VirtualArray);
2762+
DIRECT_GET_NO_DETACH_CHECK(Int64Array);
2763+
DIRECT_GET_NO_DETACH_CHECK(Uint64Array);
2764+
DIRECT_GET_NO_DETACH_CHECK(Uint8ClampedArray);
2765+
DIRECT_GET_NO_DETACH_CHECK(Uint8ClampedVirtualArray);
2766+
DIRECT_GET_VAR_CHECK_NO_DETACH_CHECK(Float32Array);
2767+
DIRECT_GET_VAR_CHECK_NO_DETACH_CHECK(Float32VirtualArray);
2768+
DIRECT_GET_VAR_CHECK_NO_DETACH_CHECK(Float64Array);
2769+
DIRECT_GET_VAR_CHECK_NO_DETACH_CHECK(Float64VirtualArray);
2770+
26712771
#define TypedArrayBeginStub(type) \
26722772
Assert(GetArrayBuffer() || GetArrayBuffer()->GetBuffer()); \
26732773
Assert(index < GetLength()); \
@@ -3196,6 +3296,14 @@ namespace Js
31963296
return GetLibrary()->GetUndefined();
31973297
}
31983298

3299+
template<>
3300+
inline Var BoolArray::DirectGetItemNoDetachCheck(__in uint32 index)
3301+
{
3302+
Assert((index + 1)* sizeof(bool) + GetByteOffset() <= GetArrayBuffer()->GetByteLength());
3303+
bool* typedBuffer = (bool*)buffer;
3304+
return typedBuffer[index] ? GetLibrary()->GetTrue() : GetLibrary()->GetFalse();
3305+
}
3306+
31993307
Var CharArray::Create(ArrayBufferBase* arrayBuffer, uint32 byteOffSet, uint32 mappedLength, JavascriptLibrary* javascriptLibrary)
32003308
{
32013309
CharArray* arr;
@@ -3285,6 +3393,16 @@ namespace Js
32853393
return FALSE;
32863394
}
32873395

3396+
inline BOOL CharArray::DirectSetItemNoDetachCheck(__in uint32 index, __in Js::Var value)
3397+
{
3398+
return DirectSetItem(index, value);
3399+
}
3400+
3401+
inline Var CharArray::DirectGetItemNoDetachCheck(__in uint32 index)
3402+
{
3403+
return DirectGetItem(index);
3404+
}
3405+
32883406
Var CharArray::TypedAdd(__in uint32 index, Var second)
32893407
{
32903408
JavascriptError::ThrowTypeError(GetScriptContext(), JSERR_InvalidOperationOnTypedArray);

‎lib/Runtime/Library/TypedArray.h‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,9 @@ namespace Js
139139
virtual BOOL DirectSetItem(__in uint32 index, __in Js::Var value) = 0;
140140
virtual BOOL DirectSetItemNoSet(__in uint32 index, __in Js::Var value) = 0;
141141
virtual Var DirectGetItem(__in uint32 index) = 0;
142+
virtual BOOL DirectSetItemNoDetachCheck(__in uint32 index, __in Js::Var value) = 0;
143+
virtual Var DirectGetItemNoDetachCheck(__in uint32 index) = 0;
144+
142145
virtual Var TypedAdd(__in uint32 index, Var second) = 0;
143146
virtual Var TypedAnd(__in uint32 index, Var second) = 0;
144147
virtual Var TypedLoad(__in uint32 index) = 0;
@@ -158,6 +161,7 @@ namespace Js
158161
static Var CommonSubarray(Arguments& args);
159162

160163
void SetObject(RecyclableObject* arraySource, uint32 targetLength, uint32 offset = 0);
164+
void SetObjectNoDetachCheck(RecyclableObject* arraySource, uint32 targetLength, uint32 offset = 0);
161165
void Set(TypedArrayBase* typedArraySource, uint32 offset = 0);
162166

163167
virtual BOOL GetDiagValueString(StringBuilder<ArenaAllocator>* stringBuilder, ScriptContext* requestContext) override;
@@ -177,6 +181,9 @@ namespace Js
177181

178182
static uint32 GetFromIndex(Var arg, uint32 length, ScriptContext *scriptContext);
179183

184+
private:
185+
uint32 GetSourceLength(RecyclableObject* arraySource, uint32 targetLength, uint32 offset);
186+
180187
protected:
181188
static Var CreateNewInstanceFromIterator(RecyclableObject *iterator, ScriptContext *scriptContext, uint32 elementSize, PFNCreateTypedArray pfnCreateTypedArray);
182189
static Var CreateNewInstance(Arguments& args, ScriptContext* scriptContext, uint32 elementSize, PFNCreateTypedArray pfnCreateTypedArray );
@@ -272,6 +279,24 @@ namespace Js
272279
return GetLibrary()->GetUndefined();
273280
}
274281

282+
inline Var BaseTypedDirectGetItemNoDetachCheck(__in uint32 index)
283+
{
284+
Assert(!IsDetachedBuffer());
285+
Assert(index < GetLength());
286+
Assert((index + 1)* sizeof(TypeName) + GetByteOffset() <= GetArrayBuffer()->GetByteLength());
287+
TypeName* typedBuffer = (TypeName*)buffer;
288+
return JavascriptNumber::ToVar(typedBuffer[index], GetScriptContext());
289+
}
290+
291+
inline Var DirectGetItemVarCheckNoDetachCheck(__in uint32 index)
292+
{
293+
Assert(!IsDetachedBuffer());
294+
Assert(index < GetLength());
295+
Assert((index + 1)* sizeof(TypeName) + GetByteOffset() <= GetArrayBuffer()->GetByteLength());
296+
TypeName* typedBuffer = (TypeName*)buffer;
297+
return JavascriptNumber::ToVarWithCheck(typedBuffer[index], GetScriptContext());
298+
}
299+
275300
inline BOOL DirectSetItemAtRange(TypedArray *fromArray, __in int32 iSrcStart, __in int32 iDstStart, __in uint32 length, TypeName(*convFunc)(Var value, ScriptContext* scriptContext))
276301
{
277302
TypeName* dstBuffer = (TypeName*)buffer;
@@ -415,10 +440,29 @@ namespace Js
415440
return FALSE;
416441
}
417442

443+
inline BOOL BaseTypedDirectSetItemNoDetachCheck(__in uint32 index, __in Js::Var value, TypeName(*convFunc)(Var value, ScriptContext* scriptContext))
444+
{
445+
TypeName typedValue = convFunc(value, GetScriptContext());
446+
447+
// The caller of the function made sure that no IsDetached check required.
448+
// The caller of the function also made sure that no length check required.
449+
450+
Assert(!IsDetachedBuffer());
451+
AssertMsg(index < GetLength(), "Trying to set out of bound index for typed array.");
452+
Assert((index + 1)* sizeof(TypeName) + GetByteOffset() <= GetArrayBuffer()->GetByteLength());
453+
TypeName* typedBuffer = (TypeName*)buffer;
454+
455+
typedBuffer[index] = typedValue;
456+
457+
return TRUE;
458+
}
459+
460+
418461
virtual BOOL DirectSetItem(__in uint32 index, __in Js::Var value) override sealed;
419462
virtual BOOL DirectSetItemNoSet(__in uint32 index, __in Js::Var value) override sealed;
420463
virtual Var DirectGetItem(__in uint32 index) override sealed;
421-
464+
virtual BOOL DirectSetItemNoDetachCheck(__in uint32 index, __in Js::Var value) override sealed;
465+
virtual Var DirectGetItemNoDetachCheck(__in uint32 index) override sealed;
422466
virtual Var TypedAdd(__in uint32 index, Var second) override;
423467
virtual Var TypedAnd(__in uint32 index, Var second) override;
424468
virtual Var TypedLoad(__in uint32 index) override;
@@ -481,6 +525,8 @@ namespace Js
481525
virtual BOOL DirectSetItem(__in uint32 index, __in Js::Var value) override;
482526
virtual BOOL DirectSetItemNoSet(__in uint32 index, __in Js::Var value) override;
483527
virtual Var DirectGetItem(__in uint32 index) override;
528+
virtual BOOL DirectSetItemNoDetachCheck(__in uint32 index, __in Js::Var value) override;
529+
virtual Var DirectGetItemNoDetachCheck(__in uint32 index) override;
484530

485531
virtual Var TypedAdd(__in uint32 index, Var second) override;
486532
virtual Var TypedAnd(__in uint32 index, Var second) override;

0 commit comments

Comments
 (0)