Skip to content

Commit 0d1e26c

Browse files
committed
[MERGE #5148 @Cellule] ArrayBuffer external memory request/free
Merge pull request #5148 from Cellule:arraybuffer_mem Charge external memory in ArrayBuffer constructor instead of all the various places where we allocate an ArrayBuffer. This is a port of the fixes I had to do in the private branch because of a conflict in ArrayBuffer with @akroshg change and #5099
2 parents 403636d + f5b2bb3 commit 0d1e26c

File tree

2 files changed

+12
-23
lines changed

2 files changed

+12
-23
lines changed

lib/Runtime/Library/ArrayBuffer.cpp

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -641,13 +641,22 @@ namespace Js
641641
}
642642
}
643643

644-
ArrayBuffer::ArrayBuffer(byte* buffer, uint32 length, DynamicType * type) :
644+
ArrayBuffer::ArrayBuffer(byte* buffer, uint32 length, DynamicType * type, bool isExternal) :
645645
buffer(buffer), bufferLength(length), ArrayBufferBase(type)
646646
{
647647
if (length > MaxArrayBufferLength)
648648
{
649649
JavascriptError::ThrowTypeError(GetScriptContext(), JSERR_FunctionArgument_Invalid);
650650
}
651+
652+
if (!isExternal)
653+
{
654+
// we take the ownership of the buffer and will have to free it so charge it to our quota.
655+
if (!this->GetRecycler()->RequestExternalMemoryAllocation(length))
656+
{
657+
JavascriptError::ThrowOutOfMemoryError(this->GetScriptContext());
658+
}
659+
}
651660
}
652661

653662
BOOL ArrayBuffer::GetDiagTypeString(StringBuilder<ArenaAllocator>* stringBuilder, ScriptContext* requestContext)
@@ -701,13 +710,6 @@ namespace Js
701710
Recycler* recycler = type->GetScriptContext()->GetRecycler();
702711
JavascriptArrayBuffer* result = RecyclerNewFinalized(recycler, JavascriptArrayBuffer, buffer, length, type);
703712
Assert(result);
704-
705-
// we take the ownership of the buffer and will have to free it so charge it to our quota.
706-
if (!recycler->RequestExternalMemoryAllocation(length))
707-
{
708-
JavascriptError::ThrowOutOfMemoryError(result->GetScriptContext());
709-
}
710-
711713
recycler->AddExternalMemoryUsage(length);
712714
return result;
713715
}
@@ -890,12 +892,6 @@ namespace Js
890892
if (buffer)
891893
{
892894
result = RecyclerNewFinalized(recycler, WebAssemblyArrayBuffer, buffer, length, type);
893-
894-
// we take the ownership of the buffer and will have to free it so charge it to our quota.
895-
if (!recycler->RequestExternalMemoryAllocation(length))
896-
{
897-
JavascriptError::ThrowOutOfMemoryError(result->GetScriptContext());
898-
}
899895
}
900896
else
901897
{
@@ -1039,13 +1035,6 @@ namespace Js
10391035
{
10401036
Recycler* recycler = type->GetScriptContext()->GetRecycler();
10411037
ProjectionArrayBuffer* result = RecyclerNewFinalized(recycler, ProjectionArrayBuffer, buffer, length, type);
1042-
1043-
// we take the ownership of the buffer and will have to free it so charge it to our quota.
1044-
if (!recycler->RequestExternalMemoryAllocation(length))
1045-
{
1046-
JavascriptError::ThrowOutOfMemoryError(result->GetScriptContext());
1047-
}
1048-
10491038
// This is user passed [in] buffer, user should AddExternalMemoryUsage before calling jscript, but
10501039
// I don't see we ask everyone to do this. Let's add the memory pressure here as well.
10511040
recycler->AddExternalMemoryUsage(length);
@@ -1072,7 +1061,7 @@ namespace Js
10721061
}
10731062

10741063
ExternalArrayBuffer::ExternalArrayBuffer(byte *buffer, uint32 length, DynamicType *type)
1075-
: ArrayBuffer(buffer, length, type)
1064+
: ArrayBuffer(buffer, length, type, true)
10761065
{
10771066
}
10781067

lib/Runtime/Library/ArrayBuffer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ namespace Js
139139
template <typename Allocator>
140140
ArrayBuffer(DECLSPEC_GUARD_OVERFLOW uint32 length, DynamicType * type, Allocator allocator);
141141

142-
ArrayBuffer(byte* buffer, DECLSPEC_GUARD_OVERFLOW uint32 length, DynamicType * type);
142+
ArrayBuffer(byte* buffer, DECLSPEC_GUARD_OVERFLOW uint32 length, DynamicType * type, bool isExternal = false);
143143

144144
class EntryInfo
145145
{

0 commit comments

Comments
 (0)