Skip to content

Commit c6af5e0

Browse files
author
Mike Kaufman
committed
[MERGE #5366 @mike-kaufman] Improving memory usage of promises
Merge pull request #5366 from mike-kaufman:build/mkaufman/promise-memory-optimizations This change improve memory usage of JavascriptPromises. On a 64-bit build, we should go from 224 bytes for a promise with one reaction to 116 bytes. Most of the gains here are by using a single SList for both reject & resolve reactions, instead of using two JsUtil::List instances. This optimization assumes that the common case is having one or two "then" calls per promise.
2 parents 5ec0ca7 + dbd57b3 commit c6af5e0

File tree

8 files changed

+157
-95
lines changed

8 files changed

+157
-95
lines changed

‎lib/Runtime/Debug/TTSnapObjects.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,21 +1075,23 @@ namespace TTD
10751075

10761076
Js::Var result = (promiseInfo->Result != nullptr) ? inflator->InflateTTDVar(promiseInfo->Result) : nullptr;
10771077

1078-
JsUtil::List<Js::JavascriptPromiseReaction*, HeapAllocator> resolveReactions(&HeapAllocator::Instance);
1078+
SList<Js::JavascriptPromiseReaction*, HeapAllocator> resolveReactions(&HeapAllocator::Instance);
10791079
for(uint32 i = 0; i < promiseInfo->ResolveReactionCount; ++i)
10801080
{
10811081
Js::JavascriptPromiseReaction* reaction = NSSnapValues::InflatePromiseReactionInfo(promiseInfo->ResolveReactions + i, ctx, inflator);
1082-
resolveReactions.Add(reaction);
1082+
resolveReactions.Prepend(reaction);
10831083
}
1084+
resolveReactions.Reverse();
10841085

1085-
JsUtil::List<Js::JavascriptPromiseReaction*, HeapAllocator> rejectReactions(&HeapAllocator::Instance);
1086+
SList<Js::JavascriptPromiseReaction*, HeapAllocator> rejectReactions(&HeapAllocator::Instance);
10861087
for(uint32 i = 0; i < promiseInfo->RejectReactionCount; ++i)
10871088
{
10881089
Js::JavascriptPromiseReaction* reaction = NSSnapValues::InflatePromiseReactionInfo(promiseInfo->RejectReactions + i, ctx, inflator);
1089-
rejectReactions.Add(reaction);
1090+
rejectReactions.Prepend(reaction);
10901091
}
1092+
rejectReactions.Reverse();
10911093

1092-
Js::RecyclableObject* res = ctx->GetLibrary()->CreatePromise_TTD(promiseInfo->Status, result, resolveReactions, rejectReactions);
1094+
Js::RecyclableObject* res = ctx->GetLibrary()->CreatePromise_TTD(promiseInfo->Status, promiseInfo->isHandled, result, resolveReactions, rejectReactions);
10931095

10941096
return res;
10951097
}
@@ -1099,6 +1101,7 @@ namespace TTD
10991101
SnapPromiseInfo* promiseInfo = SnapObjectGetAddtlInfoAs<SnapPromiseInfo*, SnapObjectType::SnapPromiseObject>(snpObject);
11001102

11011103
writer->WriteUInt32(NSTokens::Key::u32Val, promiseInfo->Status, NSTokens::Separator::CommaSeparator);
1104+
writer->WriteBool(NSTokens::Key::boolVal, promiseInfo->isHandled, NSTokens::Separator::CommaSeparator);
11021105

11031106
writer->WriteKey(NSTokens::Key::resultValue, NSTokens::Separator::CommaSeparator);
11041107
NSSnapValues::EmitTTDVar(promiseInfo->Result, writer, NSTokens::Separator::NoSeparator);
@@ -1127,6 +1130,7 @@ namespace TTD
11271130
SnapPromiseInfo* promiseInfo = alloc.SlabAllocateStruct<SnapPromiseInfo>();
11281131

11291132
promiseInfo->Status = reader->ReadUInt32(NSTokens::Key::u32Val, true);
1133+
promiseInfo->isHandled = reader->ReadBool(NSTokens::Key::boolVal, true);
11301134

11311135
reader->ReadKey(NSTokens::Key::resultValue, true);
11321136
promiseInfo->Result = NSSnapValues::ParseTTDVar(false, reader);

‎lib/Runtime/Debug/TTSnapObjects.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,7 @@ namespace TTD
371371
struct SnapPromiseInfo
372372
{
373373
uint32 Status;
374+
bool isHandled;
374375
TTDVar Result;
375376

376377
//

‎lib/Runtime/Library/JavascriptLibrary.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4993,9 +4993,9 @@ namespace Js
49934993
return JavascriptPromiseReaction::New(capabilities, handler, this->scriptContext);
49944994
}
49954995

4996-
Js::RecyclableObject* JavascriptLibrary::CreatePromise_TTD(uint32 status, Var result, JsUtil::List<Js::JavascriptPromiseReaction*, HeapAllocator>& resolveReactions, JsUtil::List<Js::JavascriptPromiseReaction*, HeapAllocator>& rejectReactions)
4996+
Js::RecyclableObject* JavascriptLibrary::CreatePromise_TTD(uint32 status, bool isHandled, Var result, SList<Js::JavascriptPromiseReaction*, HeapAllocator>& resolveReactions, SList<Js::JavascriptPromiseReaction*, HeapAllocator>& rejectReactions)
49974997
{
4998-
return JavascriptPromise::InitializePromise_TTD(this->scriptContext, status, result, resolveReactions, rejectReactions);
4998+
return JavascriptPromise::InitializePromise_TTD(this->scriptContext, status, isHandled, result, resolveReactions, rejectReactions);
49994999
}
50005000

50015001
JavascriptPromiseResolveOrRejectFunctionAlreadyResolvedWrapper* JavascriptLibrary::CreateAlreadyDefinedWrapper_TTD(bool alreadyDefined)

‎lib/Runtime/Library/JavascriptLibrary.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ namespace Js
643643
Js::JavascriptPromiseCapability* CreatePromiseCapability_TTD(Var promise, Var resolve, Var reject);
644644
Js::JavascriptPromiseReaction* CreatePromiseReaction_TTD(RecyclableObject* handler, JavascriptPromiseCapability* capabilities);
645645

646-
Js::RecyclableObject* CreatePromise_TTD(uint32 status, Var result, JsUtil::List<Js::JavascriptPromiseReaction*, HeapAllocator>& resolveReactions, JsUtil::List<Js::JavascriptPromiseReaction*, HeapAllocator>& rejectReactions);
646+
Js::RecyclableObject* CreatePromise_TTD(uint32 status, bool isHandled, Var result, SList<Js::JavascriptPromiseReaction*, HeapAllocator>& resolveReactions, SList<Js::JavascriptPromiseReaction*, HeapAllocator>& rejectReactions);
647647
JavascriptPromiseResolveOrRejectFunctionAlreadyResolvedWrapper* CreateAlreadyDefinedWrapper_TTD(bool alreadyDefined);
648648
Js::RecyclableObject* CreatePromiseResolveOrRejectFunction_TTD(RecyclableObject* promise, bool isReject, JavascriptPromiseResolveOrRejectFunctionAlreadyResolvedWrapper* alreadyResolved);
649649
Js::RecyclableObject* CreatePromiseReactionTaskFunction_TTD(JavascriptPromiseReaction* reaction, Var argument);

‎lib/Runtime/Library/JavascriptPromise.cpp

Lines changed: 93 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,14 @@
77
namespace Js
88
{
99
JavascriptPromise::JavascriptPromise(DynamicType * type)
10-
: DynamicObject(type)
10+
: DynamicObject(type),
11+
isHandled(false),
12+
status(PromiseStatus::PromiseStatusCode_Undefined),
13+
result(nullptr),
14+
reactions(nullptr)
15+
1116
{
1217
Assert(type->GetTypeId() == TypeIds_Promise);
13-
14-
this->status = PromiseStatusCode_Undefined;
15-
this->isHandled = false;
16-
this->result = nullptr;
17-
this->resolveReactions = nullptr;
18-
this->rejectReactions = nullptr;
1918
}
2019

2120
// Promise() as defined by ES 2016 Sections 25.4.3.1
@@ -100,17 +99,16 @@ namespace Js
10099

101100
void JavascriptPromise::InitializePromise(JavascriptPromise* promise, JavascriptPromiseResolveOrRejectFunction** resolve, JavascriptPromiseResolveOrRejectFunction** reject, ScriptContext* scriptContext)
102101
{
103-
Assert(promise->status == PromiseStatusCode_Undefined);
102+
Assert(promise->GetStatus() == PromiseStatusCode_Undefined);
104103
Assert(resolve);
105104
Assert(reject);
106105

107106
Recycler* recycler = scriptContext->GetRecycler();
108107
JavascriptLibrary* library = scriptContext->GetLibrary();
109108

110-
promise->status = PromiseStatusCode_Unresolved;
109+
promise->SetStatus(PromiseStatusCode_Unresolved);
111110

112-
promise->resolveReactions = RecyclerNew(recycler, JavascriptPromiseReactionList, recycler);
113-
promise->rejectReactions = RecyclerNew(recycler, JavascriptPromiseReactionList, recycler);
111+
promise->reactions = RecyclerNew(recycler, JavascriptPromiseReactionList, recycler);
114112

115113
JavascriptPromiseResolveOrRejectFunctionAlreadyResolvedWrapper* alreadyResolvedRecord = RecyclerNewStructZ(scriptContext->GetRecycler(), JavascriptPromiseResolveOrRejectFunctionAlreadyResolvedWrapper);
116114
alreadyResolvedRecord->alreadyResolved = false;
@@ -151,14 +149,9 @@ namespace Js
151149
return TRUE;
152150
}
153151

154-
JavascriptPromiseReactionList* JavascriptPromise::GetResolveReactions()
155-
{
156-
return this->resolveReactions;
157-
}
158-
159-
JavascriptPromiseReactionList* JavascriptPromise::GetRejectReactions()
152+
JavascriptPromiseReactionList* JavascriptPromise::GetReactions()
160153
{
161-
return this->rejectReactions;
154+
return this->reactions;
162155
}
163156

164157
// Promise.all as described in ES 2015 Section 25.4.4.1
@@ -848,13 +841,12 @@ namespace Js
848841
}
849842
}
850843

851-
JavascriptPromiseReactionList* reactions;
844+
852845
PromiseStatus newStatus;
853846

854847
// Need to check rejecting state again as it might have changed due to failures
855848
if (isRejecting)
856849
{
857-
reactions = this->GetRejectReactions();
858850
newStatus = PromiseStatusCode_HasRejection;
859851
if (!GetIsHandled())
860852
{
@@ -863,18 +855,24 @@ namespace Js
863855
}
864856
else
865857
{
866-
reactions = this->GetResolveReactions();
867858
newStatus = PromiseStatusCode_HasResolution;
868859
}
869860

870861
Assert(resolution != nullptr);
871862

863+
// SList only supports "prepend" operation, so we need to reverse the list
864+
// before triggering reactions
865+
JavascriptPromiseReactionList* reactions = this->GetReactions();
866+
if (reactions != nullptr)
867+
{
868+
reactions->Reverse();
869+
}
870+
872871
this->result = resolution;
873-
this->resolveReactions = nullptr;
874-
this->rejectReactions = nullptr;
875-
this->status = newStatus;
872+
this->reactions = nullptr;
873+
this->SetStatus(newStatus);
876874

877-
return TriggerPromiseReactions(reactions, resolution, scriptContext);
875+
return TriggerPromiseReactions(reactions, isRejecting, resolution, scriptContext);
878876
}
879877

880878
// Promise Capabilities Executor Function as described in ES 2015 Section 25.4.1.6.2
@@ -1001,10 +999,12 @@ namespace Js
1001999
{
10021000
JavascriptPromise * curr = stack.Pop();
10031001
{
1004-
JavascriptPromiseReactionList* reactions = curr->GetRejectReactions();
1005-
for (int i = 0; i < reactions->Count(); i++)
1002+
JavascriptPromiseReactionList* reactions = curr->GetReactions();
1003+
JavascriptPromiseReactionList::Iterator it = reactions->GetIterator();
1004+
while (it.Next())
10061005
{
1007-
JavascriptPromiseReaction* reaction = reactions->Item(i);
1006+
JavascriptPromiseReactionPair pair = it.Data();
1007+
JavascriptPromiseReaction* reaction = pair.rejectReaction;
10081008
Var promiseVar = reaction->GetCapabilities()->GetPromise();
10091009

10101010
if (JavascriptPromise::Is(promiseVar))
@@ -1132,11 +1132,14 @@ namespace Js
11321132
JavascriptPromiseReaction* resolveReaction = JavascriptPromiseReaction::New(promiseCapability, fulfillmentHandler, scriptContext);
11331133
JavascriptPromiseReaction* rejectReaction = JavascriptPromiseReaction::New(promiseCapability, rejectionHandler, scriptContext);
11341134

1135-
switch (sourcePromise->status)
1135+
switch (sourcePromise->GetStatus())
11361136
{
11371137
case PromiseStatusCode_Unresolved:
1138-
sourcePromise->resolveReactions->Add(resolveReaction);
1139-
sourcePromise->rejectReactions->Add(rejectReaction);
1138+
JavascriptPromiseReactionPair pair;
1139+
pair.resolveReaction = resolveReaction;
1140+
pair.rejectReaction = rejectReaction;
1141+
1142+
sourcePromise->reactions->Prepend(pair);
11401143
break;
11411144
case PromiseStatusCode_HasResolution:
11421145
EnqueuePromiseReactionTask(resolveReaction, sourcePromise->result, scriptContext);
@@ -1478,20 +1481,12 @@ namespace Js
14781481
extractor->MarkVisitVar(this->result);
14791482
}
14801483

1481-
if(this->resolveReactions != nullptr)
1482-
{
1483-
for(int32 i = 0; i < this->resolveReactions->Count(); ++i)
1484-
{
1485-
this->resolveReactions->Item(i)->MarkVisitPtrs(extractor);
1486-
}
1487-
}
1488-
1489-
if(this->rejectReactions != nullptr)
1484+
if(this->reactions != nullptr)
14901485
{
1491-
for(int32 i = 0; i < this->rejectReactions->Count(); ++i)
1492-
{
1493-
this->rejectReactions->Item(i)->MarkVisitPtrs(extractor);
1494-
}
1486+
this->reactions->Map([&](JavascriptPromiseReactionPair pair) {
1487+
pair.rejectReaction->MarkVisitPtrs(extractor);
1488+
pair.resolveReaction->MarkVisitPtrs(extractor);
1489+
});
14951490
}
14961491
}
14971492

@@ -1514,29 +1509,34 @@ namespace Js
15141509
depOnList.Add(TTD_CONVERT_VAR_TO_PTR_ID(this->result));
15151510
}
15161511

1517-
spi->Status = this->status;
1512+
spi->Status = this->GetStatus();
1513+
spi->isHandled = this->GetIsHandled();
15181514

1519-
spi->ResolveReactionCount = (this->resolveReactions != nullptr) ? this->resolveReactions->Count() : 0;
1520-
spi->ResolveReactions = nullptr;
1521-
if(spi->ResolveReactionCount != 0)
1515+
// get count of # of reactions
1516+
spi->ResolveReactionCount = 0;
1517+
if (this->reactions != nullptr)
15221518
{
1523-
spi->ResolveReactions = alloc.SlabAllocateArray<TTD::NSSnapValues::SnapPromiseReactionInfo>(spi->ResolveReactionCount);
1524-
1525-
for(uint32 i = 0; i < spi->ResolveReactionCount; ++i)
1526-
{
1527-
this->resolveReactions->Item(i)->ExtractSnapPromiseReactionInto(spi->ResolveReactions + i, depOnList, alloc);
1528-
}
1519+
this->reactions->Map([&spi](JavascriptPromiseReactionPair pair) {
1520+
spi->ResolveReactionCount++;
1521+
});
15291522
}
1523+
spi->RejectReactionCount = spi->ResolveReactionCount;
15301524

1531-
spi->RejectReactionCount = (this->rejectReactions != nullptr) ? this->rejectReactions->Count() : 0;
1525+
// move resolve & reject reactions into slab
1526+
spi->ResolveReactions = nullptr;
15321527
spi->RejectReactions = nullptr;
1533-
if(spi->RejectReactionCount != 0)
1528+
if(spi->ResolveReactionCount != 0)
15341529
{
1530+
spi->ResolveReactions = alloc.SlabAllocateArray<TTD::NSSnapValues::SnapPromiseReactionInfo>(spi->ResolveReactionCount);
15351531
spi->RejectReactions = alloc.SlabAllocateArray<TTD::NSSnapValues::SnapPromiseReactionInfo>(spi->RejectReactionCount);
15361532

1537-
for(uint32 i = 0; i < spi->RejectReactionCount; ++i)
1533+
JavascriptPromiseReactionList::Iterator it = this->reactions->GetIterator();
1534+
uint32 i = 0;
1535+
while (it.Next())
15381536
{
1539-
this->rejectReactions->Item(i)->ExtractSnapPromiseReactionInto(spi->RejectReactions+ i, depOnList, alloc);
1537+
it.Data().resolveReaction->ExtractSnapPromiseReactionInto(spi->ResolveReactions + i, depOnList, alloc);
1538+
it.Data().rejectReaction->ExtractSnapPromiseReactionInto(spi->RejectReactions + i, depOnList, alloc);
1539+
++i;
15401540
}
15411541
}
15421542

@@ -1559,22 +1559,38 @@ namespace Js
15591559
}
15601560
}
15611561

1562-
JavascriptPromise* JavascriptPromise::InitializePromise_TTD(ScriptContext* scriptContext, uint32 status, Var result, JsUtil::List<Js::JavascriptPromiseReaction*, HeapAllocator>& resolveReactions, JsUtil::List<Js::JavascriptPromiseReaction*, HeapAllocator>& rejectReactions)
1562+
JavascriptPromise* JavascriptPromise::InitializePromise_TTD(ScriptContext* scriptContext, uint32 status, bool isHandled, Var result, SList<Js::JavascriptPromiseReaction*, HeapAllocator>& resolveReactions,SList<Js::JavascriptPromiseReaction*, HeapAllocator>& rejectReactions)
15631563
{
15641564
Recycler* recycler = scriptContext->GetRecycler();
15651565
JavascriptLibrary* library = scriptContext->GetLibrary();
15661566

15671567
JavascriptPromise* promise = library->CreatePromise();
15681568

1569-
promise->status = (PromiseStatus)status;
1569+
promise->SetStatus((PromiseStatus)status);
1570+
if (isHandled)
1571+
{
1572+
promise->SetIsHandled();
1573+
}
15701574
promise->result = result;
15711575

1572-
promise->resolveReactions = RecyclerNew(recycler, JavascriptPromiseReactionList, recycler);
1573-
promise->resolveReactions->Copy(&resolveReactions);
1574-
1575-
promise->rejectReactions = RecyclerNew(recycler, JavascriptPromiseReactionList, recycler);
1576-
promise->rejectReactions->Copy(&rejectReactions);
1576+
promise->reactions = RecyclerNew(recycler, JavascriptPromiseReactionList, recycler);
1577+
SList<Js::JavascriptPromiseReaction*, HeapAllocator>::Iterator resolveIterator = resolveReactions.GetIterator();
1578+
SList<Js::JavascriptPromiseReaction*, HeapAllocator>::Iterator rejectIterator = rejectReactions.GetIterator();
15771579

1580+
bool hasResolve = resolveIterator.Next();
1581+
bool hasReject = rejectIterator.Next();
1582+
while (hasResolve && hasReject)
1583+
{
1584+
JavascriptPromiseReactionPair pair;
1585+
pair.resolveReaction = resolveIterator.Data();
1586+
pair.rejectReaction = rejectIterator.Data();
1587+
promise->reactions->Prepend(pair);
1588+
hasResolve = resolveIterator.Next();
1589+
hasReject = rejectIterator.Next();
1590+
}
1591+
AssertMsg(hasResolve == false && hasReject == false, "mismatched resolve/reject reaction counts");
1592+
promise->reactions->Reverse();
1593+
15781594
return promise;
15791595
}
15801596
#endif
@@ -1621,15 +1637,24 @@ namespace Js
16211637
}
16221638

16231639
// TriggerPromiseReactions as defined in ES 2015 Section 25.4.1.7
1624-
Var JavascriptPromise::TriggerPromiseReactions(JavascriptPromiseReactionList* reactions, Var resolution, ScriptContext* scriptContext)
1640+
Var JavascriptPromise::TriggerPromiseReactions(JavascriptPromiseReactionList* reactions, bool isRejecting, Var resolution, ScriptContext* scriptContext)
16251641
{
16261642
JavascriptLibrary* library = scriptContext->GetLibrary();
16271643

16281644
if (reactions != nullptr)
16291645
{
1630-
for (int i = 0; i < reactions->Count(); i++)
1646+
JavascriptPromiseReactionList::Iterator it = reactions->GetIterator();
1647+
while (it.Next())
16311648
{
1632-
JavascriptPromiseReaction* reaction = reactions->Item(i);
1649+
JavascriptPromiseReaction* reaction;
1650+
if (isRejecting)
1651+
{
1652+
reaction = it.Data().rejectReaction;
1653+
}
1654+
else
1655+
{
1656+
reaction = it.Data().resolveReaction;
1657+
}
16331658

16341659
EnqueuePromiseReactionTask(reaction, resolution, scriptContext);
16351660
}

0 commit comments

Comments
 (0)