From 31c9d83acbd9806ebd312df3f02b77cb259ad250 Mon Sep 17 00:00:00 2001 From: "jst%netscape.com" Date: Tue, 11 Sep 2001 00:55:05 +0000 Subject: [PATCH] Fixing bug 98828. Cache a few nsSpaceManager objects to avoid spending 43%cvs ci -m ! of the time spent in PresShell::ProcessReflowCommand() in new and delete. r=dbaron@fas.harvard.edu, sr=waterson@netscape.com --- layout/base/src/nsSpaceManager.cpp | 85 ++++++++++++++++++++- layout/base/src/nsSpaceManager.h | 16 +++- layout/build/nsLayoutModule.cpp | 3 + layout/generic/nsAreaFrame.cpp | 2 - layout/generic/nsAreaFrame.h | 2 - layout/generic/nsBlockFrame.cpp | 2 +- layout/generic/nsSpaceManager.cpp | 85 ++++++++++++++++++++- layout/generic/nsSpaceManager.h | 16 +++- layout/html/base/src/nsAreaFrame.cpp | 2 - layout/html/base/src/nsAreaFrame.h | 2 - layout/html/base/src/nsBlockFrame.cpp | 2 +- layout/xul/base/src/nsBoxToBlockAdaptor.cpp | 2 +- layout/xul/base/src/nsContainerBox.cpp | 1 - 13 files changed, 200 insertions(+), 20 deletions(-) diff --git a/layout/base/src/nsSpaceManager.cpp b/layout/base/src/nsSpaceManager.cpp index a2ffdf8e3ee6..3b22ce7ca7bf 100644 --- a/layout/base/src/nsSpaceManager.cpp +++ b/layout/base/src/nsSpaceManager.cpp @@ -35,6 +35,10 @@ ///////////////////////////////////////////////////////////////////////////// // BandList +PRInt32 nsSpaceManager::sCachedSpaceManagerCount = 0; +nsSpaceManager * + nsSpaceManager::sCachedSpaceManagers[NS_SPACE_MANAGER_CACHE_SIZE]; + #define NSCOORD_MIN (-2147483647 - 1) /* minimum signed value */ nsSpaceManager::BandList::BandList() @@ -88,7 +92,86 @@ nsSpaceManager::~nsSpaceManager() ClearFrameInfo(); } -NS_IMPL_ISUPPORTS1(nsSpaceManager, nsISpaceManager) + +NS_INTERFACE_MAP_BEGIN(nsSpaceManager) + NS_INTERFACE_MAP_ENTRY(nsISpaceManager) + NS_INTERFACE_MAP_ENTRY(nsISupports) +NS_INTERFACE_MAP_END + +NS_IMPL_ADDREF(nsSpaceManager) +NS_IMPL_RELEASE_WITH_DESTROY(nsSpaceManager, LastRelease()) + + +// static +nsSpaceManager *nsSpaceManager::Create(nsIFrame* aFrame) +{ + if (sCachedSpaceManagerCount > 0) { + // We have cached unused instances of this class, return a cached + // instance in stead of always creating a new one. + nsSpaceManager *spaceManager = + sCachedSpaceManagers[--sCachedSpaceManagerCount]; + + // Re-initialize the cached space manager by calling its + // constructor (using placement new), the destructor was called + // when the space manager was put in the cache. + return new (spaceManager) nsSpaceManager(aFrame); + } + + // The cache is empty, this means we haveto create a new instance. + return new nsSpaceManager(aFrame); +} + + +// static +void nsSpaceManager::Shutdown() +{ + // The layout module is being shut down, clean up the cache and + // disable further caching. + + PRInt32 i; + + for (i = 0; i < sCachedSpaceManagerCount; i++) { + // The destructor for the cached space managers has already been + // called (when the space manager was put in the cache) so we cast + // spaceManager to char * when calling delete to prevent the + // destructor from being called again. + + nsSpaceManager *spaceManager = sCachedSpaceManagers[i]; + + delete (char *)spaceManager; + } + + // Disable futher caching. + sCachedSpaceManagerCount = -1; +} + + +void +nsSpaceManager::LastRelease() +{ + // This space manager is no longer used, if there's still room in + // the cache we'll cache this space manager, unless the layout + // module was already shut down. + + if (sCachedSpaceManagerCount < NS_SPACE_MANAGER_CACHE_SIZE && + sCachedSpaceManagerCount >= 0) { + // There's still space in the cache for more instances, put this + // instance in the cache in stead of deleting it. + + sCachedSpaceManagers[sCachedSpaceManagerCount++] = this; + + // Call the destructor so that the proper cleanup happens + this->~nsSpaceManager(); + + return; + } + + // The cache is full, or the layout module has been shut down, + // delete this space manager. + + delete this; +} + NS_IMETHODIMP nsSpaceManager::GetFrame(nsIFrame*& aFrame) const diff --git a/layout/base/src/nsSpaceManager.h b/layout/base/src/nsSpaceManager.h index f77da2ea9f85..0f57a73193f6 100644 --- a/layout/base/src/nsSpaceManager.h +++ b/layout/base/src/nsSpaceManager.h @@ -25,13 +25,16 @@ #include "nsISpaceManager.h" #include "prclist.h" +#define NS_SPACE_MANAGER_CACHE_SIZE 4 + /** * Implementation of nsISpaceManager that maintains a region data structure of * unavailable space */ class nsSpaceManager : public nsISpaceManager { public: - nsSpaceManager(nsIFrame* aFrame); + static nsSpaceManager *Create(nsIFrame* aFrame); + static void Shutdown(); // nsISupports NS_DECL_ISUPPORTS @@ -64,6 +67,8 @@ public: #endif protected: + nsSpaceManager(nsIFrame* aFrame); + // Structure that maintains information about the region associated // with a particular frame struct FrameInfo { @@ -169,8 +174,13 @@ protected: nsBandData& aAvailableSpace) const; private: - nsSpaceManager(const nsSpaceManager&); // no implementation - void operator=(const nsSpaceManager&); // no implementation + static PRInt32 sCachedSpaceManagerCount; + static nsSpaceManager *sCachedSpaceManagers[NS_SPACE_MANAGER_CACHE_SIZE]; + + void LastRelease(); + + nsSpaceManager(const nsSpaceManager&); // no implementation + void operator=(const nsSpaceManager&); // no implementation }; #endif /* nsSpaceManager_h___ */ diff --git a/layout/build/nsLayoutModule.cpp b/layout/build/nsLayoutModule.cpp index e73e78f21230..8f80d6697e5d 100644 --- a/layout/build/nsLayoutModule.cpp +++ b/layout/build/nsLayoutModule.cpp @@ -44,6 +44,7 @@ #include "nsCSSAtoms.h" // to addref/release table #include "nsColorNames.h" // to addref/release table #include "nsCSSFrameConstructor.h" +#include "nsSpaceManager.h" #ifdef INCLUDE_XUL #include "nsXULAtoms.h" @@ -146,6 +147,8 @@ Shutdown(nsIModule* self) nsCSSFrameConstructor::ReleaseGlobals(); nsTextTransformer::Shutdown(); + + nsSpaceManager::Shutdown(); } #ifdef NS_DEBUG diff --git a/layout/generic/nsAreaFrame.cpp b/layout/generic/nsAreaFrame.cpp index dade5ed46a6d..145ff8c58282 100644 --- a/layout/generic/nsAreaFrame.cpp +++ b/layout/generic/nsAreaFrame.cpp @@ -26,7 +26,6 @@ #include "nsStyleConsts.h" #include "nsIPresContext.h" #include "nsIViewManager.h" -#include "nsSpaceManager.h" #include "nsHTMLAtoms.h" #include "nsIView.h" #include "nsHTMLIIDs.h" @@ -36,7 +35,6 @@ #include "nsISizeOfHandler.h" #undef NOISY_MAX_ELEMENT_SIZE -#undef NOISY_SPACEMANAGER #undef NOISY_FINAL_SIZE nsresult diff --git a/layout/generic/nsAreaFrame.h b/layout/generic/nsAreaFrame.h index ac03e52d4631..21654d977ad2 100644 --- a/layout/generic/nsAreaFrame.h +++ b/layout/generic/nsAreaFrame.h @@ -26,8 +26,6 @@ #include "nsVoidArray.h" #include "nsAbsoluteContainingBlock.h" -class nsSpaceManager; - struct nsStyleDisplay; struct nsStylePosition; diff --git a/layout/generic/nsBlockFrame.cpp b/layout/generic/nsBlockFrame.cpp index f28a2d141be3..e4d8713ed6fc 100644 --- a/layout/generic/nsBlockFrame.cpp +++ b/layout/generic/nsBlockFrame.cpp @@ -674,7 +674,7 @@ nsBlockFrame::Reflow(nsIPresContext* aPresContext, // only when there are actually floats to manage. Otherwise things // like tables will gain significant bloat. if (NS_BLOCK_SPACE_MGR & mState) { - nsSpaceManager* rawPtr = new nsSpaceManager(this); + nsSpaceManager* rawPtr = nsSpaceManager::Create(this); if (!rawPtr) { return NS_ERROR_OUT_OF_MEMORY; } diff --git a/layout/generic/nsSpaceManager.cpp b/layout/generic/nsSpaceManager.cpp index a2ffdf8e3ee6..3b22ce7ca7bf 100644 --- a/layout/generic/nsSpaceManager.cpp +++ b/layout/generic/nsSpaceManager.cpp @@ -35,6 +35,10 @@ ///////////////////////////////////////////////////////////////////////////// // BandList +PRInt32 nsSpaceManager::sCachedSpaceManagerCount = 0; +nsSpaceManager * + nsSpaceManager::sCachedSpaceManagers[NS_SPACE_MANAGER_CACHE_SIZE]; + #define NSCOORD_MIN (-2147483647 - 1) /* minimum signed value */ nsSpaceManager::BandList::BandList() @@ -88,7 +92,86 @@ nsSpaceManager::~nsSpaceManager() ClearFrameInfo(); } -NS_IMPL_ISUPPORTS1(nsSpaceManager, nsISpaceManager) + +NS_INTERFACE_MAP_BEGIN(nsSpaceManager) + NS_INTERFACE_MAP_ENTRY(nsISpaceManager) + NS_INTERFACE_MAP_ENTRY(nsISupports) +NS_INTERFACE_MAP_END + +NS_IMPL_ADDREF(nsSpaceManager) +NS_IMPL_RELEASE_WITH_DESTROY(nsSpaceManager, LastRelease()) + + +// static +nsSpaceManager *nsSpaceManager::Create(nsIFrame* aFrame) +{ + if (sCachedSpaceManagerCount > 0) { + // We have cached unused instances of this class, return a cached + // instance in stead of always creating a new one. + nsSpaceManager *spaceManager = + sCachedSpaceManagers[--sCachedSpaceManagerCount]; + + // Re-initialize the cached space manager by calling its + // constructor (using placement new), the destructor was called + // when the space manager was put in the cache. + return new (spaceManager) nsSpaceManager(aFrame); + } + + // The cache is empty, this means we haveto create a new instance. + return new nsSpaceManager(aFrame); +} + + +// static +void nsSpaceManager::Shutdown() +{ + // The layout module is being shut down, clean up the cache and + // disable further caching. + + PRInt32 i; + + for (i = 0; i < sCachedSpaceManagerCount; i++) { + // The destructor for the cached space managers has already been + // called (when the space manager was put in the cache) so we cast + // spaceManager to char * when calling delete to prevent the + // destructor from being called again. + + nsSpaceManager *spaceManager = sCachedSpaceManagers[i]; + + delete (char *)spaceManager; + } + + // Disable futher caching. + sCachedSpaceManagerCount = -1; +} + + +void +nsSpaceManager::LastRelease() +{ + // This space manager is no longer used, if there's still room in + // the cache we'll cache this space manager, unless the layout + // module was already shut down. + + if (sCachedSpaceManagerCount < NS_SPACE_MANAGER_CACHE_SIZE && + sCachedSpaceManagerCount >= 0) { + // There's still space in the cache for more instances, put this + // instance in the cache in stead of deleting it. + + sCachedSpaceManagers[sCachedSpaceManagerCount++] = this; + + // Call the destructor so that the proper cleanup happens + this->~nsSpaceManager(); + + return; + } + + // The cache is full, or the layout module has been shut down, + // delete this space manager. + + delete this; +} + NS_IMETHODIMP nsSpaceManager::GetFrame(nsIFrame*& aFrame) const diff --git a/layout/generic/nsSpaceManager.h b/layout/generic/nsSpaceManager.h index f77da2ea9f85..0f57a73193f6 100644 --- a/layout/generic/nsSpaceManager.h +++ b/layout/generic/nsSpaceManager.h @@ -25,13 +25,16 @@ #include "nsISpaceManager.h" #include "prclist.h" +#define NS_SPACE_MANAGER_CACHE_SIZE 4 + /** * Implementation of nsISpaceManager that maintains a region data structure of * unavailable space */ class nsSpaceManager : public nsISpaceManager { public: - nsSpaceManager(nsIFrame* aFrame); + static nsSpaceManager *Create(nsIFrame* aFrame); + static void Shutdown(); // nsISupports NS_DECL_ISUPPORTS @@ -64,6 +67,8 @@ public: #endif protected: + nsSpaceManager(nsIFrame* aFrame); + // Structure that maintains information about the region associated // with a particular frame struct FrameInfo { @@ -169,8 +174,13 @@ protected: nsBandData& aAvailableSpace) const; private: - nsSpaceManager(const nsSpaceManager&); // no implementation - void operator=(const nsSpaceManager&); // no implementation + static PRInt32 sCachedSpaceManagerCount; + static nsSpaceManager *sCachedSpaceManagers[NS_SPACE_MANAGER_CACHE_SIZE]; + + void LastRelease(); + + nsSpaceManager(const nsSpaceManager&); // no implementation + void operator=(const nsSpaceManager&); // no implementation }; #endif /* nsSpaceManager_h___ */ diff --git a/layout/html/base/src/nsAreaFrame.cpp b/layout/html/base/src/nsAreaFrame.cpp index dade5ed46a6d..145ff8c58282 100644 --- a/layout/html/base/src/nsAreaFrame.cpp +++ b/layout/html/base/src/nsAreaFrame.cpp @@ -26,7 +26,6 @@ #include "nsStyleConsts.h" #include "nsIPresContext.h" #include "nsIViewManager.h" -#include "nsSpaceManager.h" #include "nsHTMLAtoms.h" #include "nsIView.h" #include "nsHTMLIIDs.h" @@ -36,7 +35,6 @@ #include "nsISizeOfHandler.h" #undef NOISY_MAX_ELEMENT_SIZE -#undef NOISY_SPACEMANAGER #undef NOISY_FINAL_SIZE nsresult diff --git a/layout/html/base/src/nsAreaFrame.h b/layout/html/base/src/nsAreaFrame.h index ac03e52d4631..21654d977ad2 100644 --- a/layout/html/base/src/nsAreaFrame.h +++ b/layout/html/base/src/nsAreaFrame.h @@ -26,8 +26,6 @@ #include "nsVoidArray.h" #include "nsAbsoluteContainingBlock.h" -class nsSpaceManager; - struct nsStyleDisplay; struct nsStylePosition; diff --git a/layout/html/base/src/nsBlockFrame.cpp b/layout/html/base/src/nsBlockFrame.cpp index f28a2d141be3..e4d8713ed6fc 100644 --- a/layout/html/base/src/nsBlockFrame.cpp +++ b/layout/html/base/src/nsBlockFrame.cpp @@ -674,7 +674,7 @@ nsBlockFrame::Reflow(nsIPresContext* aPresContext, // only when there are actually floats to manage. Otherwise things // like tables will gain significant bloat. if (NS_BLOCK_SPACE_MGR & mState) { - nsSpaceManager* rawPtr = new nsSpaceManager(this); + nsSpaceManager* rawPtr = nsSpaceManager::Create(this); if (!rawPtr) { return NS_ERROR_OUT_OF_MEMORY; } diff --git a/layout/xul/base/src/nsBoxToBlockAdaptor.cpp b/layout/xul/base/src/nsBoxToBlockAdaptor.cpp index 358d0c3aa8ee..71915a60460f 100644 --- a/layout/xul/base/src/nsBoxToBlockAdaptor.cpp +++ b/layout/xul/base/src/nsBoxToBlockAdaptor.cpp @@ -609,7 +609,7 @@ nsBoxToBlockAdaptor::Reflow(nsBoxLayoutState& aState, // printf("In debug\n"); if (mSpaceManager == nsnull) { - mSpaceManager = new nsSpaceManager(mFrame); + mSpaceManager = nsSpaceManager::Create(mFrame); NS_ADDREF(mSpaceManager); } diff --git a/layout/xul/base/src/nsContainerBox.cpp b/layout/xul/base/src/nsContainerBox.cpp index bc0d3a9b8b21..bea7f171c61f 100644 --- a/layout/xul/base/src/nsContainerBox.cpp +++ b/layout/xul/base/src/nsContainerBox.cpp @@ -53,7 +53,6 @@ #include "nsXULAtoms.h" #include "nsIReflowCommand.h" #include "nsIContent.h" -#include "nsSpaceManager.h" #include "nsHTMLParts.h" #include "nsIViewManager.h" #include "nsIView.h"