Bug 1470771: Part 1 - Don't evict shared bundles from the bundle cache. r=erahm
MozReview-Commit-ID: 6BLN4hOH5fW
--- a/intl/strres/nsStringBundle.cpp
+++ b/intl/strres/nsStringBundle.cpp
@@ -675,17 +675,17 @@ NS_IMPL_ISUPPORTS(nsStringBundleService,
nsIStringBundleService,
nsIObserver,
nsISupportsWeakReference,
nsIMemoryReporter)
nsStringBundleService::~nsStringBundleService()
{
UnregisterWeakMemoryReporter(this);
- flushBundleCache();
+ flushBundleCache(false);
}
nsresult
nsStringBundleService::Init()
{
nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
if (os) {
os->AddObserver(this, "memory-pressure", true);
@@ -710,48 +710,51 @@ nsStringBundleService::SizeOfIncludingTh
return aMallocSizeOf(this) + n;
}
NS_IMETHODIMP
nsStringBundleService::Observe(nsISupports* aSubject,
const char* aTopic,
const char16_t* aSomeData)
{
- if (strcmp("memory-pressure", aTopic) == 0 ||
- strcmp("profile-do-change", aTopic) == 0 ||
+ if (strcmp("profile-do-change", aTopic) == 0 ||
strcmp("chrome-flush-caches", aTopic) == 0 ||
strcmp("intl:app-locales-changed", aTopic) == 0)
{
- flushBundleCache();
+ flushBundleCache(false);
+ } else if (strcmp("memory-pressure", aTopic) == 0) {
+ flushBundleCache(true);
}
return NS_OK;
}
void
-nsStringBundleService::flushBundleCache()
+nsStringBundleService::flushBundleCache(bool ignoreShared)
{
- // release all bundles in the cache
- mBundleMap.Clear();
+ LinkedList<bundleCacheEntry_t> newList;
while (!mBundleCache.isEmpty()) {
- delete mBundleCache.popFirst();
+ UniquePtr<bundleCacheEntry_t> entry(mBundleCache.popFirst());
+ auto* bundle = nsStringBundleBase::Cast(entry->mBundle);
+
+ if (ignoreShared && bundle->IsShared()) {
+ newList.insertBack(entry.release());
+ } else {
+ mBundleMap.Remove(entry->mHashKey);
+ }
}
- // We never flush shared bundles, since their memory cannot be freed, so add
- // them back to the map.
- for (auto* entry : mSharedBundles) {
- mBundleMap.Put(entry->mHashKey, entry);
- }
+ mBundleCache = std::move(newList);
}
NS_IMETHODIMP
nsStringBundleService::FlushBundles()
{
- flushBundleCache();
+ flushBundleCache(false);
return NS_OK;
}
void
nsStringBundleService::SendContentBundles(ContentParent* aContentParent) const
{
nsTArray<StringBundleDescriptor> bundles;
@@ -800,24 +803,21 @@ nsStringBundleService::getStringBundle(c
nsIStringBundle **aResult)
{
nsDependentCString key(aURLSpec);
bundleCacheEntry_t* cacheEntry = mBundleMap.Get(key);
RefPtr<SharedStringBundle> shared;
if (cacheEntry) {
- // cache hit!
- // remove it from the list, it will later be reinserted
- // at the head of the list
+ // Remove the entry from the list so it can be re-inserted at the back.
cacheEntry->remove();
shared = do_QueryObject(cacheEntry->mBundle);
} else {
- // hasn't been cached, so insert it into the hash table
nsCOMPtr<nsIStringBundle> bundle;
bool isContent = IsContentBundle(key);
if (!isContent || !XRE_IsParentProcess()) {
bundle = new nsStringBundle(aURLSpec);
}
// If this is a bundle which is used by the content processes, we want to
// load it into a shared memory region.
@@ -842,58 +842,58 @@ nsStringBundleService::getStringBundle(c
}
cacheEntry = insertIntoCache(bundle.forget(), key);
}
if (shared) {
mSharedBundles.insertBack(cacheEntry);
} else {
- // at this point the cacheEntry should exist in the hashtable,
- // but is not in the LRU cache.
- // put the cache entry at the front of the list
- mBundleCache.insertFront(cacheEntry);
+ mBundleCache.insertBack(cacheEntry);
}
// finally, return the value
*aResult = cacheEntry->mBundle;
NS_ADDREF(*aResult);
}
-bundleCacheEntry_t *
+UniquePtr<bundleCacheEntry_t>
+nsStringBundleService::evictOneEntry()
+{
+ for (auto* entry : mBundleCache) {
+ auto* bundle = nsStringBundleBase::Cast(entry->mBundle);
+ if (!bundle->IsShared()) {
+ entry->remove();
+ mBundleMap.Remove(entry->mHashKey);
+ return UniquePtr<bundleCacheEntry_t>(entry);
+ }
+ }
+ return nullptr;
+}
+
+bundleCacheEntry_t*
nsStringBundleService::insertIntoCache(already_AddRefed<nsIStringBundle> aBundle,
const nsACString& aHashKey)
{
- bundleCacheEntry_t *cacheEntry;
+ UniquePtr<bundleCacheEntry_t> cacheEntry;
- if (mBundleMap.Count() < MAX_CACHED_BUNDLES ||
- mBundleCache.isEmpty()) {
- // cache not full - create a new entry
- cacheEntry = new bundleCacheEntry_t();
- } else {
- // cache is full
- // take the last entry in the list, and recycle it.
- cacheEntry = mBundleCache.getLast();
-
- // remove it from the hash table and linked list
- NS_ASSERTION(mBundleMap.Contains(cacheEntry->mHashKey),
- "Element will not be removed!");
- mBundleMap.Remove(cacheEntry->mHashKey);
- cacheEntry->remove();
+ if (mBundleMap.Count() >= MAX_CACHED_BUNDLES) {
+ cacheEntry = evictOneEntry();
}
- // at this point we have a new cacheEntry that doesn't exist
- // in the hashtable, so set up the cacheEntry
+ if (!cacheEntry) {
+ cacheEntry.reset(new bundleCacheEntry_t());
+ }
+
cacheEntry->mHashKey = aHashKey;
cacheEntry->mBundle = aBundle;
- // insert the entry into the cache and map, make it the MRU
- mBundleMap.Put(cacheEntry->mHashKey, cacheEntry);
+ mBundleMap.Put(cacheEntry->mHashKey, cacheEntry.get());
- return cacheEntry;
+ return cacheEntry.release();
}
NS_IMETHODIMP
nsStringBundleService::CreateBundle(const char* aURLSpec,
nsIStringBundle** aResult)
{
getStringBundle(aURLSpec,aResult);
return NS_OK;
--- a/intl/strres/nsStringBundle.h
+++ b/intl/strres/nsStringBundle.h
@@ -23,16 +23,25 @@ public:
NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_NSISTRINGBUNDLE
virtual nsresult LoadProperties() = 0;
const nsCString& BundleURL() const { return mPropertiesURL; }
+ // Returns true if this bundle has more than one reference. If it has only
+ // a single reference, it is assumed to be held alive by the bundle cache.
+ bool IsShared() const { return mRefCnt > 1; }
+
+ static nsStringBundleBase* Cast(nsIStringBundle* aBundle)
+ {
+ return static_cast<nsStringBundleBase*>(aBundle);
+ }
+
protected:
virtual ~nsStringBundleBase();
virtual nsresult GetStringImpl(const nsACString& aName, nsAString& aResult) = 0;
virtual nsresult GetSimpleEnumerationImpl(nsISimpleEnumerator** elements) = 0;
nsCString mPropertiesURL;
--- a/intl/strres/nsStringBundleService.h
+++ b/intl/strres/nsStringBundleService.h
@@ -12,16 +12,17 @@
#include "nsIPersistentProperties2.h"
#include "nsIStringBundle.h"
#include "nsIObserver.h"
#include "nsWeakReference.h"
#include "nsIErrorService.h"
#include "nsIMemoryReporter.h"
#include "mozilla/LinkedList.h"
+#include "mozilla/UniquePtr.h"
struct bundleCacheEntry_t;
class nsStringBundleService : public nsIStringBundleService,
public nsIObserver,
public nsSupportsWeakReference,
public nsIMemoryReporter
{
@@ -59,21 +60,25 @@ public:
private:
virtual ~nsStringBundleService();
void getStringBundle(const char *aUrl, nsIStringBundle** aResult);
nsresult FormatWithBundle(nsIStringBundle* bundle, nsresult aStatus,
uint32_t argCount, char16_t** argArray,
nsAString& result);
- void flushBundleCache();
+ void flushBundleCache(bool ignoreShared = true);
+
+ mozilla::UniquePtr<bundleCacheEntry_t> evictOneEntry();
bundleCacheEntry_t* insertIntoCache(already_AddRefed<nsIStringBundle> aBundle,
const nsACString &aHashKey);
nsDataHashtable<nsCStringHashKey, bundleCacheEntry_t*> mBundleMap;
+ // LRU list of cached entries, with the least-recently-used entry first.
mozilla::LinkedList<bundleCacheEntry_t> mBundleCache;
+ // List of cached shared-memory string bundles, in arbitrary order.
mozilla::AutoCleanLinkedList<bundleCacheEntry_t> mSharedBundles;
nsCOMPtr<nsIErrorService> mErrorService;
};
#endif
--- a/js/xpconnect/loader/AutoMemMap.cpp
+++ b/js/xpconnect/loader/AutoMemMap.cpp
@@ -101,22 +101,16 @@ AutoMemMap::init(const FileDescriptor& f
MOZ_ASSERT(!fd);
MOZ_ASSERT(!handle_);
if (!file.IsValid()) {
return Err(NS_ERROR_INVALID_ARG);
}
handle_ = file.ClonePlatformHandle().release();
- return initInternal(prot, size);
-}
-
-Result<Ok, nsresult>
-AutoMemMap::initInternal(PRFileMapProtect prot, size_t size)
-{
MOZ_ASSERT(!addr);
size_ = size;
addr = MapViewOfFile(
handle_,
prot == PR_PROT_READONLY ? FILE_MAP_READ : FILE_MAP_ALL_ACCESS,
0, 0, size);
@@ -133,22 +127,16 @@ AutoMemMap::cloneHandle() const
#else
Result<Ok, nsresult>
AutoMemMap::init(const FileDescriptor& file, size_t size, PRFileMapProtect prot)
{
return init(file, prot);
}
-Result<Ok, nsresult>
-AutoMemMap::initInternal(PRFileMapProtect prot, size_t size)
-{
- return initInternal(prot);
-}
-
FileDescriptor
AutoMemMap::cloneHandle() const
{
return cloneFileDescriptor();
}
#endif
--- a/js/xpconnect/loader/AutoMemMap.h
+++ b/js/xpconnect/loader/AutoMemMap.h
@@ -73,18 +73,16 @@ class AutoMemMap
// Makes this mapping persistent. After calling this, the mapped memory
// will remained mapped, even after this instance is destroyed.
void setPersistent() { persistent_ = true; }
private:
Result<Ok, nsresult> initInternal(PRFileMapProtect prot = PR_PROT_READONLY,
size_t expectedSize = 0);
- Result<Ok, nsresult> initInternal(PRFileMapProtect prot, size_t size);
-
AutoFDClose fd;
PRFileMap* fileMap = nullptr;
#ifdef XP_WIN
// We can't include windows.h in this header, since it gets included
// by some binding headers (which are explicitly incompatible with
// windows.h). So we can't use the HANDLE type here.
void* handle_ = nullptr;