This change provides additional performance improvements to the fined…

… grained PCP changelists that were added in 2342828.

When processing changes for large stages, the PcpChanges::_didChangeSpecsInternal set could grow very large.  A performance gain can be realized by performing a more restrive dependency search and storing only topmost specs that changed in a new set.  When applying changes to the cache, the prim index is used to determine the full set of specs to update.

Additionally when muting and unmuting layers, we also perform a more restrive dependency search by only recusing on site paths.

(Internal change: 2343722)

pxr/usd/pcp/cache.cpp

@@ -1073,6 +1073,13 @@ PcpCache::Apply(const PcpCacheChanges& changes, PcpLifeboat* lifeboat)
             updateSpecStacks(*i);
         }
 
+        TF_FOR_ALL(i, changes._didChangeSpecsAndChildrenInternal) {
+            auto range = _primIndexCache.FindSubtreeRange(*i);
+            for (auto i = range.first; i != range.second; ++i) {
+                updateSpecStacks(i->first);
+            }
+        }
+
         // Fix the keys for any prim or property under any of the renamed
         // paths.
         // XXX: It'd be nice if this was a usd by just adjusting

pxr/usd/pcp/changes.cpp

@@ -1446,13 +1446,15 @@ void PcpChanges::DidMuteAndUnmuteLayers(
     }
 }
 
-void PcpChanges::_ProcessLayerStackAndDependencyChanges(
+void 
+PcpChanges::_MarkReferencingSitesAsSignificantlyChanged(
     const PcpCache* cache,
-    const PcpLayerStackPtrVector& layerStacks,
-    bool markRefsOrPayloadSitesAsChanged)
+    const PcpLayerStackPtrVector& layerStacks)
 {
     TRACE_FUNCTION();
 
+    const PcpCacheChanges& cacheChanges = _GetCacheChanges(cache);
+
     const auto refOrPayloadChangeFunc = 
         [this, cache](const SdfPath &depIndexPath, 
                         const PcpNodeRef &node)
@@ -1465,40 +1467,59 @@ void PcpChanges::_ProcessLayerStackAndDependencyChanges(
             }
         };
 
-
     for (const PcpLayerStackPtr& layerStack : layerStacks) {
-        _DidChangeLayerStack(cache,
-                             layerStack,
-                             /*requiresLayerStackChange*/ true,
-                             /*requiresLayerStackOffsetsChange*/ false,
-                             /*requiresSignificantChange*/ true);
-
         PcpDependencyVector deps = cache->FindSiteDependencies(
             layerStack,
             SdfPath::AbsoluteRootPath(), 
             PcpDependencyTypeAnyIncludingVirtual,
             /* recurseOnSite */ true,
-            /* recurseOnIndex */ true,
+            /* recurseOnIndex */ false,
             /* filter */ true);
 
         for(const PcpDependency &dep: deps) {
             // This ensures that all sites which reference this layer are
             // also marked as having changed significantly.
-            if (markRefsOrPayloadSitesAsChanged && 
-                PcpComposeSiteHasPrimSpecs(layerStack, dep.sitePath))
+            if (PcpComposeSiteHasPrimSpecs(
+                layerStack, dep.sitePath, cacheChanges.layersToMute))
             {
                 Pcp_ForEachDependentNode(dep.sitePath, layerStack,
                                         dep.indexPath,
                                         *cache, refOrPayloadChangeFunc);
             }
+        }
+    }
+}
+
+void 
+PcpChanges::_ProcessLayerStackAndDependencyChanges(
+    const PcpCache* cache,
+    const PcpLayerStackPtrVector& layerStacks)
+{
+    TRACE_FUNCTION();
 
+    for (const PcpLayerStackPtr& layerStack : layerStacks) {
+        _DidChangeLayerStack(cache,
+                             layerStack,
+                             /*requiresLayerStackChange*/ true,
+                             /*requiresLayerStackOffsetsChange*/ false,
+                             /*requiresSignificantChange*/ true);
+
+        PcpDependencyVector deps = cache->FindSiteDependencies(
+            layerStack,
+            SdfPath::AbsoluteRootPath(), 
+            PcpDependencyTypeAnyIncludingVirtual,
+            /* recurseOnSite */ false,
+            /* recurseOnIndex */ false,
+            /* filter */ true);
+
+        for(const PcpDependency &dep: deps) {
             // We also need to mark dependencies spec stacks as changed due to
             // the fact that the addition or removal of layers will result in
             // the need of prim stack indicies to be updated.
             // Note that property indexes don't have to be updated because they
             // hold on to spec objects directly instead of being index-based.
             if (dep.indexPath.IsAbsoluteRootOrPrimPath()) {
-                _DidChangeSpecStackInternal(cache, dep.indexPath);
+                _DidChangeSpecStackAndChildrenInternal(cache, dep.indexPath);
             }
         }
     }
@@ -1549,8 +1570,8 @@ PcpChanges::_DidMuteLayer(
             std::move(changes.front()));
         _lifeboat.Retain(mutedLayer);
 
-        _ProcessLayerStackAndDependencyChanges(cache, layerStacks, 
-            /*markRefsOrPayloadSitesAsChanged*/ true);
+        _ProcessLayerStackAndDependencyChanges(cache, layerStacks);
+        _MarkReferencingSitesAsSignificantlyChanged(cache, layerStacks);
     }
 
     cacheChanges.didMuteOrUnmuteNonEmptyLayer |= 
@@ -1612,8 +1633,8 @@ PcpChanges::_DidUnmuteLayer(
             std::move(changes.front()));
         _lifeboat.Retain(unmutedLayer);
 
-        _ProcessLayerStackAndDependencyChanges(cache, layerStacks, 
-            /*markRefsOrPayloadSitesAsChanged*/ true);
+        _ProcessLayerStackAndDependencyChanges(cache, layerStacks);
+        _MarkReferencingSitesAsSignificantlyChanged(cache, layerStacks);
     }
 
     cacheChanges.didMuteOrUnmuteNonEmptyLayer |= 
@@ -2218,8 +2239,7 @@ PcpChanges::_DidAddOrRemoveSublayer(
                     empty, /*compareFieldValues*/ false)));
             }
 
-            _ProcessLayerStackAndDependencyChanges(cache, layerStacks, 
-                /*markRefsOrPayloadSitesAsChanged*/ false);
+            _ProcessLayerStackAndDependencyChanges(cache, layerStacks);
 
             DidChange(cache, changes);
 
@@ -2835,4 +2855,11 @@ PcpChanges::_DidChangeSpecStackInternal(
     _GetCacheChanges(cache)._didChangeSpecsInternal.insert(path);
 }
 
+void 
+PcpChanges::_DidChangeSpecStackAndChildrenInternal(
+    const PcpCache* cache, const SdfPath& path)
+{
+    _GetCacheChanges(cache)._didChangeSpecsAndChildrenInternal.insert(path);
+}
+
 PXR_NAMESPACE_CLOSE_SCOPE

pxr/usd/pcp/changes.h

@@ -150,6 +150,13 @@ class PcpCacheChanges {
     // not its contents.  Because this causes no externally-observable
     // changes in state, clients do not need to be aware of these changes.
     SdfPathSet _didChangeSpecsInternal;
+
+    // This set serves a similar purpose to _didChangeSpecsInternal above,
+    // however, during processing descendants of the specs in this set will also
+    // be marked as changed. A performance gain is accomplished by placing the
+    // ancestor specs in this set and marking children iteratively when applying
+    // changes to the cache.
+    SdfPathSet _didChangeSpecsAndChildrenInternal;
 };
 
 /// Structure used to temporarily retain layers and layerStacks within
@@ -436,17 +443,24 @@ class PcpChanges {
     void _DidChangeSpecStackInternal(
         const PcpCache* cache, const SdfPath& path);
 
-    // During change processing for added/removed or muted/unmuted layers,
-    // layer stacks and dependant paths need to be marked as having changed
-    // so that relevant data can be recalculated when the cache is applied.
-    // The \p markRefsOrPayloadSitesAsChanged parameter controls if, when
-    // traversing dependencies, if one is introduced via a reference or
-    // payload then that site will be marked as having changed significantly.
-    // This is set when processing changes for muted and unmuted layers.
+    void _DidChangeSpecStackAndChildrenInternal(
+        const PcpCache* cache, const SdfPath& path);
+
+    // This method is used when processing layer operations.  It ensures that
+    // affected layer stacks and their dependent spec stacks are marked as
+    // changed.
     void _ProcessLayerStackAndDependencyChanges(
         const PcpCache* cache,
-        const PcpLayerStackPtrVector& layerStacks,
-        bool markRefsOrPayloadSitesAsChanged);
+        const PcpLayerStackPtrVector& layerStacks);
+
+    // When muting or unmuting a layer that is being referenced or payloaded,
+    // we need to ensure that all the relevant sites are recomposed.  This
+    // function searches site dependencies of the provided layer stacks and
+    // marks those that are introduced via reference or payload arcs as
+    // significantly changed.
+    void _MarkReferencingSitesAsSignificantlyChanged(
+        const PcpCache* cache,
+        const PcpLayerStackPtrVector& layerStacks);
 
 private:
     LayerStackChanges _layerStackChanges;