From 8c46126007e39319e1e8631468a54d078f315361 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Thu, 22 Jul 2021 23:26:36 -0400 Subject: [PATCH] Suppress vault callbacks when loading or unloading VNodeMgrs. On MOULa, there have been complaints about linking to some Neighborhoods and activating some players taking a very long time. One of these Neighborhoods in particular is the "DRC(67) Bevin". When I attempted to link to this Age, I found the link took approximately 85 seconds each time. On profiling, I discovered that for every node downloaded during the initialization phase, we were calling into Python at least once. Suppressing vault callbacks during times when they are obviously going to storm and be useless decreases the link time to 7 seconds. --- .../PubUtilLib/plVault/plVaultClientApi.cpp | 85 +++++++++++++------ .../PubUtilLib/plVault/plVaultClientApi.h | 12 +++ 2 files changed, 73 insertions(+), 24 deletions(-) diff --git a/Sources/Plasma/PubUtilLib/plVault/plVaultClientApi.cpp b/Sources/Plasma/PubUtilLib/plVault/plVaultClientApi.cpp index bbb4e014..98b4eb03 100644 --- a/Sources/Plasma/PubUtilLib/plVault/plVaultClientApi.cpp +++ b/Sources/Plasma/PubUtilLib/plVault/plVaultClientApi.cpp @@ -181,7 +181,28 @@ struct VaultDownloadTrans { unsigned vaultId; ENetError result; - VaultDownloadTrans (); + VaultDownloadTrans() + : callback(), cbParam(), progressCallback(), cbProgressParam(), + nodeCount(), nodesLeft(), vaultId(), result(kNetSuccess) + { + VaultSuppressCallbacks(); + } + + VaultDownloadTrans(const wchar_t* _tag, FVaultDownloadCallback _callback, + void* _cbParam, FVaultProgressCallback _progressCallback, + void* _cbProgressParam, unsigned _vaultId) + : callback(_callback), cbParam(_cbParam), progressCallback(_progressCallback), + cbProgressParam(_cbProgressParam), nodeCount(), nodesLeft(), + vaultId(_vaultId), result(kNetSuccess) + { + StrCopy(tag, _tag, MAX_PATH); + VaultSuppressCallbacks(); + } + + ~VaultDownloadTrans() + { + VaultEnableCallbacks(); + } static void VaultNodeFetched ( ENetError result, @@ -262,6 +283,8 @@ static HASHTABLEDECL( static bool s_processPlayerInbox = false; +static long s_suppressCallbacks = 0; + /***************************************************************************** * * Local functions @@ -303,8 +326,10 @@ static void VaultNodeAddedDownloadCallback(ENetError result, void * param) { VaultProcessUnvisitNote(childLink->node); } - for (IVaultCallback * cb = s_callbacks.Head(); cb; cb = s_callbacks.Next(cb)) - cb->cb->AddedChildNode(parentLink->node, childLink->node); + if (s_suppressCallbacks == 0) { + for (IVaultCallback* cb = s_callbacks.Head(); cb; cb = s_callbacks.Next(cb)) + cb->cb->AddedChildNode(parentLink->node, childLink->node); + } } } @@ -377,9 +402,11 @@ static void BuildNodeTree ( parentNode->state->children.Add(childLink); if (notifyNow || childNode->nodeType != 0) { - // We made a new link, so make the callbacks - for (IVaultCallback * cb = s_callbacks.Head(); cb; cb = s_callbacks.Next(cb)) - cb->cb->AddedChildNode(parentNode, childNode); + if (s_suppressCallbacks == 0) { + // We made a new link, so make the callbacks + for (IVaultCallback* cb = s_callbacks.Head(); cb; cb = s_callbacks.Next(cb)) + cb->cb->AddedChildNode(parentNode, childNode); + } } else { INotifyAfterDownload* notify = NEWZERO(INotifyAfterDownload)(parentNode->nodeId, childNode->nodeId); @@ -543,6 +570,10 @@ static void ChangedVaultNodeFetched ( RelVaultNodeLink* savedLink = s_nodes.Find(node->nodeId); + // Yeah, we are purposefully allowing this global callback to go out, + // even if callback suppression has been enabled. The intent behind + // that is to suppress spurious callbacks, but node changes are + // probably not spurious. if (savedLink) { for (IVaultCallback * cb = s_callbacks.Head(); cb; cb = s_callbacks.Next(cb)) cb->cb->ChangedNode(savedLink->node); @@ -638,7 +669,7 @@ static void VaultNodeAdded ( RelVaultNodeLink* parentLink = s_nodes.Find(parentId); RelVaultNodeLink* childLink = s_nodes.Find(childId); - if (childLink->node->nodeType != 0) { + if (childLink->node->nodeType != 0 && s_suppressCallbacks == 0) { for (IVaultCallback * cb = s_callbacks.Head(); cb; cb = s_callbacks.Next(cb)) cb->cb->AddedChildNode(parentLink->node, childLink->node); } @@ -660,7 +691,7 @@ static void VaultNodeRemoved ( if (!childLink) break; - if (parentLink->node->IsParentOf(childId, 1)) { + if (parentLink->node->IsParentOf(childId, 1) && s_suppressCallbacks == 0) { // We have the relationship, so make the callbacks for (IVaultCallback * cb = s_callbacks.Head(); cb; cb = s_callbacks.Next(cb)) cb->cb->RemovingChildNode(parentLink->node, childLink->node); @@ -831,11 +862,6 @@ void VaultFindNodeTrans::VaultNodeFound ( * ***/ -//============================================================================ -VaultDownloadTrans::VaultDownloadTrans () { - ASSERT(!nodeCount); // must be alloced with -} - //============================================================================ void VaultDownloadTrans::VaultNodeFetched ( ENetError result, @@ -1047,8 +1073,10 @@ void IRelVaultNode::UnlinkFromRelatives () { next = parents.Next(link); // We have the relationship, so make the callbacks - for (IVaultCallback * cb = s_callbacks.Head(); cb; cb = s_callbacks.Next(cb)) - cb->cb->RemovingChildNode(link->node, this->node); + if (s_suppressCallbacks == 0) { + for (IVaultCallback* cb = s_callbacks.Head(); cb; cb = s_callbacks.Next(cb)) + cb->cb->RemovingChildNode(link->node, this->node); + } link->node->state->Unlink(node); } @@ -1478,6 +1506,16 @@ void VaultUnregisterCallback (VaultCallback * cb) { cb->internal = nil; } +//============================================================================ +void VaultSuppressCallbacks() { + AtomicAdd(&s_suppressCallbacks, 1); +} + +//============================================================================ +void VaultEnableCallbacks() { + long prevValue = AtomicAdd(&s_suppressCallbacks, -1); + hsAssert(prevValue > 0, "Hmm... A negative vault callback suppression count?"); +} /***************************************************************************** * @@ -1772,8 +1810,10 @@ void VaultRemoveChildNode ( if (parentLink->node->IsParentOf(childId, 1)) { // We have the relationship, so make the callbacks - for (IVaultCallback * cb = s_callbacks.Head(); cb; cb = s_callbacks.Next(cb)) - cb->cb->RemovingChildNode(parentLink->node, childLink->node); + if (s_suppressCallbacks == 0) { + for (IVaultCallback* cb = s_callbacks.Head(); cb; cb = s_callbacks.Next(cb)) + cb->cb->RemovingChildNode(parentLink->node, childLink->node); + } } parentLink->node->state->Unlink(childLink->node); @@ -4945,13 +4985,8 @@ void VaultDownload ( FVaultProgressCallback progressCallback, void * cbProgressParam ) { - VaultDownloadTrans * trans = NEWZERO(VaultDownloadTrans); - StrCopy(trans->tag, tag, arrsize(trans->tag)); - trans->callback = callback; - trans->cbParam = cbParam; - trans->progressCallback = progressCallback; - trans->cbProgressParam = cbProgressParam; - trans->vaultId = vaultId; + VaultDownloadTrans * trans = new VaultDownloadTrans(tag, callback, cbParam, + progressCallback, cbProgressParam, vaultId); NetCliAuthVaultFetchNodeRefs( vaultId, @@ -5001,6 +5036,8 @@ void VaultDownloadAndWait ( //============================================================================ void VaultCull (unsigned vaultId) { + VaultCallbackSuppressor suppress; + // Remove the node from the global table if (RelVaultNodeLink * link = s_nodes.Find(vaultId)) { LogMsg(kLogDebug, L"Vault: Culling node %u", link->node->nodeId); diff --git a/Sources/Plasma/PubUtilLib/plVault/plVaultClientApi.h b/Sources/Plasma/PubUtilLib/plVault/plVaultClientApi.h index 39bd0aa0..3ecd968b 100644 --- a/Sources/Plasma/PubUtilLib/plVault/plVaultClientApi.h +++ b/Sources/Plasma/PubUtilLib/plVault/plVaultClientApi.h @@ -82,6 +82,18 @@ struct VaultCallback { void VaultRegisterCallback (VaultCallback * cb); void VaultUnregisterCallback (VaultCallback * cb); +void VaultSuppressCallbacks(); +void VaultEnableCallbacks(); + +class VaultCallbackSuppressor +{ + VaultCallbackSuppressor(const VaultCallbackSuppressor&) { } + VaultCallbackSuppressor(VaultCallbackSuppressor&&) { } + +public: + VaultCallbackSuppressor() { VaultSuppressCallbacks(); } + ~VaultCallbackSuppressor() { VaultEnableCallbacks(); } +}; /***************************************************************************** *