Commit 31faa713 authored by Tanmoy Mollik's avatar Tanmoy Mollik Committed by Commit Bot

Renaming memory_used() in cached_storage_area.cc to better reflect what the function does

The memory_used() function in cached_storage_area.cc actually returns quota_used by storage
area map which is 2 times the size in memory. It is renamed to quota_used() to better
reflect what it does and a new memory_used() function has been added to return the actual
memory used.

For context see this cl: https://chromium-review.googlesource.com/c/chromium/src/+/1541242/

Bug: None
Change-Id: I05ad7ae51528283c239512ee0be4f30f66861c56
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1596452
Commit-Queue: Tanmoy Mollik <triploblastic@google.com>
Reviewed-by: default avatarMarijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657330}
parent f9f331cd
...@@ -369,11 +369,8 @@ bool CachedStorageArea::OnMemoryDump( ...@@ -369,11 +369,8 @@ bool CachedStorageArea::OnMemoryDump(
IsSessionStorage() ? "session_storage" : "local_storage", IsSessionStorage() ? "session_storage" : "local_storage",
reinterpret_cast<uintptr_t>(this)); reinterpret_cast<uintptr_t>(this));
MemoryAllocatorDump* dump = pmd->CreateAllocatorDump(dump_name.Utf8().data()); MemoryAllocatorDump* dump = pmd->CreateAllocatorDump(dump_name.Utf8().data());
// TODO(triploblastic@): rename memory_used() to quota_used() and create
// memory_used() function to actually count the memory used.
dump->AddScalar(MemoryAllocatorDump::kNameSize, dump->AddScalar(MemoryAllocatorDump::kNameSize,
MemoryAllocatorDump::kUnitsBytes, MemoryAllocatorDump::kUnitsBytes, memory_used());
memory_used() / sizeof(UChar));
pmd->AddSuballocation( pmd->AddSuballocation(
dump->guid(), dump->guid(),
String(WTF::Partitions::kAllocatedObjectPoolName).Utf8().data()); String(WTF::Partitions::kAllocatedObjectPoolName).Utf8().data());
......
...@@ -86,7 +86,8 @@ class MODULES_EXPORT CachedStorageArea ...@@ -86,7 +86,8 @@ class MODULES_EXPORT CachedStorageArea
// Returns the (unique) id allocated for this source for testing purposes. // Returns the (unique) id allocated for this source for testing purposes.
String RegisterSource(Source* source); String RegisterSource(Source* source);
size_t memory_used() const { return map_ ? map_->quota_used() : 0; } size_t quota_used() const { return map_ ? map_->quota_used() : 0; }
size_t memory_used() const { return map_ ? map_->memory_used() : 0; }
// Only public to allow tests to parametrize on this type. // Only public to allow tests to parametrize on this type.
enum class FormatOption { enum class FormatOption {
......
...@@ -13,6 +13,10 @@ size_t QuotaForString(const String& s) { ...@@ -13,6 +13,10 @@ size_t QuotaForString(const String& s) {
return s.length() * sizeof(UChar); return s.length() * sizeof(UChar);
} }
size_t MemoryForString(const String& s) {
return s.CharactersSizeInBytes();
}
} // namespace } // namespace
StorageAreaMap::StorageAreaMap(size_t quota) : quota_(quota) { StorageAreaMap::StorageAreaMap(size_t quota) : quota_(quota) {
...@@ -77,6 +81,7 @@ bool StorageAreaMap::RemoveItem(const String& key, String* old_value) { ...@@ -77,6 +81,7 @@ bool StorageAreaMap::RemoveItem(const String& key, String* old_value) {
if (it == keys_values_.end()) if (it == keys_values_.end())
return false; return false;
quota_used_ -= QuotaForString(key) + QuotaForString(it->value); quota_used_ -= QuotaForString(key) + QuotaForString(it->value);
memory_used_ -= MemoryForString(key) + MemoryForString(it->value);
if (old_value) if (old_value)
*old_value = it->value; *old_value = it->value;
keys_values_.erase(it); keys_values_.erase(it);
...@@ -95,14 +100,18 @@ bool StorageAreaMap::SetItemInternal(const String& key, ...@@ -95,14 +100,18 @@ bool StorageAreaMap::SetItemInternal(const String& key,
bool check_quota) { bool check_quota) {
const auto it = keys_values_.find(key); const auto it = keys_values_.find(key);
size_t old_item_size = 0; size_t old_item_size = 0;
size_t old_item_memory = 0;
if (it != keys_values_.end()) { if (it != keys_values_.end()) {
old_item_size = QuotaForString(key) + QuotaForString(it->value); old_item_size = QuotaForString(key) + QuotaForString(it->value);
old_item_memory = MemoryForString(key) + MemoryForString(it->value);
if (old_value) if (old_value)
*old_value = it->value; *old_value = it->value;
} }
DCHECK_GE(quota_used_, old_item_size); DCHECK_GE(quota_used_, old_item_size);
size_t new_item_size = QuotaForString(key) + QuotaForString(value); size_t new_item_size = QuotaForString(key) + QuotaForString(value);
size_t new_item_memory = MemoryForString(key) + MemoryForString(value);
size_t new_quota_used = quota_used_ - old_item_size + new_item_size; size_t new_quota_used = quota_used_ - old_item_size + new_item_size;
size_t new_memory_used = memory_used_ - old_item_memory + new_item_memory;
// Only check quota if the size is increasing, this allows // Only check quota if the size is increasing, this allows
// shrinking changes to pre-existing files that are over budget. // shrinking changes to pre-existing files that are over budget.
...@@ -112,6 +121,7 @@ bool StorageAreaMap::SetItemInternal(const String& key, ...@@ -112,6 +121,7 @@ bool StorageAreaMap::SetItemInternal(const String& key,
keys_values_.Set(key, value); keys_values_.Set(key, value);
ResetKeyIterator(); ResetKeyIterator();
quota_used_ = new_quota_used; quota_used_ = new_quota_used;
memory_used_ = new_memory_used;
return true; return true;
} }
......
...@@ -38,6 +38,7 @@ class MODULES_EXPORT StorageAreaMap { ...@@ -38,6 +38,7 @@ class MODULES_EXPORT StorageAreaMap {
bool RemoveItem(const String& key, String* old_value); bool RemoveItem(const String& key, String* old_value);
size_t quota_used() const { return quota_used_; } size_t quota_used() const { return quota_used_; }
size_t memory_used() const { return memory_used_; }
size_t quota() const { return quota_; } size_t quota() const { return quota_; }
private: private:
...@@ -55,6 +56,7 @@ class MODULES_EXPORT StorageAreaMap { ...@@ -55,6 +56,7 @@ class MODULES_EXPORT StorageAreaMap {
mutable unsigned last_key_index_; mutable unsigned last_key_index_;
size_t quota_used_ = 0; size_t quota_used_ = 0;
size_t memory_used_ = 0;
const size_t quota_; const size_t quota_;
}; };
......
...@@ -79,7 +79,7 @@ TEST(StorageControllerTest, CacheLimit) { ...@@ -79,7 +79,7 @@ TEST(StorageControllerTest, CacheLimit) {
cached_area1->SetItem(kKey, kValue, source_area); cached_area1->SetItem(kKey, kValue, source_area);
const auto* area1_ptr = cached_area1.get(); const auto* area1_ptr = cached_area1.get();
size_t expected_total = (kKey.length() + kValue.length()) * 2; size_t expected_total = (kKey.length() + kValue.length()) * 2;
EXPECT_EQ(expected_total, cached_area1->memory_used()); EXPECT_EQ(expected_total, cached_area1->quota_used());
EXPECT_EQ(expected_total, controller.TotalCacheSize()); EXPECT_EQ(expected_total, controller.TotalCacheSize());
cached_area1 = nullptr; cached_area1 = nullptr;
...@@ -87,14 +87,14 @@ TEST(StorageControllerTest, CacheLimit) { ...@@ -87,14 +87,14 @@ TEST(StorageControllerTest, CacheLimit) {
cached_area2->RegisterSource(source_area); cached_area2->RegisterSource(source_area);
cached_area2->SetItem(kKey, kValue, source_area); cached_area2->SetItem(kKey, kValue, source_area);
// Area for kOrigin should still be alive. // Area for kOrigin should still be alive.
EXPECT_EQ(2 * cached_area2->memory_used(), controller.TotalCacheSize()); EXPECT_EQ(2 * cached_area2->quota_used(), controller.TotalCacheSize());
EXPECT_EQ(area1_ptr, controller.GetLocalStorageArea(kOrigin.get())); EXPECT_EQ(area1_ptr, controller.GetLocalStorageArea(kOrigin.get()));
String long_value(Vector<UChar>(kTestCacheLimit, 'a')); String long_value(Vector<UChar>(kTestCacheLimit, 'a'));
cached_area2->SetItem(kKey, long_value, source_area); cached_area2->SetItem(kKey, long_value, source_area);
// Cache is cleared when a new area is opened. // Cache is cleared when a new area is opened.
auto cached_area3 = controller.GetLocalStorageArea(kOrigin3.get()); auto cached_area3 = controller.GetLocalStorageArea(kOrigin3.get());
EXPECT_EQ(cached_area2->memory_used(), controller.TotalCacheSize()); EXPECT_EQ(cached_area2->quota_used(), controller.TotalCacheSize());
} }
TEST(StorageControllerTest, CacheLimitSessionStorage) { TEST(StorageControllerTest, CacheLimitSessionStorage) {
...@@ -143,7 +143,7 @@ TEST(StorageControllerTest, CacheLimitSessionStorage) { ...@@ -143,7 +143,7 @@ TEST(StorageControllerTest, CacheLimitSessionStorage) {
cached_area1->SetItem(kKey, kValue, source_area); cached_area1->SetItem(kKey, kValue, source_area);
const auto* area1_ptr = cached_area1.get(); const auto* area1_ptr = cached_area1.get();
size_t expected_total = (kKey.length() + kValue.length()) * 2; size_t expected_total = (kKey.length() + kValue.length()) * 2;
EXPECT_EQ(expected_total, cached_area1->memory_used()); EXPECT_EQ(expected_total, cached_area1->quota_used());
EXPECT_EQ(expected_total, controller.TotalCacheSize()); EXPECT_EQ(expected_total, controller.TotalCacheSize());
cached_area1 = nullptr; cached_area1 = nullptr;
...@@ -151,14 +151,14 @@ TEST(StorageControllerTest, CacheLimitSessionStorage) { ...@@ -151,14 +151,14 @@ TEST(StorageControllerTest, CacheLimitSessionStorage) {
cached_area2->RegisterSource(source_area); cached_area2->RegisterSource(source_area);
cached_area2->SetItem(kKey, kValue, source_area); cached_area2->SetItem(kKey, kValue, source_area);
// Area for kOrigin should still be alive. // Area for kOrigin should still be alive.
EXPECT_EQ(2 * cached_area2->memory_used(), controller.TotalCacheSize()); EXPECT_EQ(2 * cached_area2->quota_used(), controller.TotalCacheSize());
EXPECT_EQ(area1_ptr, ns1->GetCachedArea(kOrigin.get())); EXPECT_EQ(area1_ptr, ns1->GetCachedArea(kOrigin.get()));
String long_value(Vector<UChar>(kTestCacheLimit, 'a')); String long_value(Vector<UChar>(kTestCacheLimit, 'a'));
cached_area2->SetItem(kKey, long_value, source_area); cached_area2->SetItem(kKey, long_value, source_area);
// Cache is cleared when a new area is opened. // Cache is cleared when a new area is opened.
auto cached_area3 = ns1->GetCachedArea(kOrigin3.get()); auto cached_area3 = ns1->GetCachedArea(kOrigin3.get());
EXPECT_EQ(cached_area2->memory_used(), controller.TotalCacheSize()); EXPECT_EQ(cached_area2->quota_used(), controller.TotalCacheSize());
int32_t opens = 0; int32_t opens = 0;
{ {
......
...@@ -140,7 +140,7 @@ void StorageNamespace::CloneTo(const String& target) { ...@@ -140,7 +140,7 @@ void StorageNamespace::CloneTo(const String& target) {
size_t StorageNamespace::TotalCacheSize() const { size_t StorageNamespace::TotalCacheSize() const {
size_t total = 0; size_t total = 0;
for (const auto& it : cached_areas_) for (const auto& it : cached_areas_)
total += it.value->memory_used(); total += it.value->quota_used();
return total; return total;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment