1191650 - Bug in ReportStorageMemory()?

Status: RESOLVED INVALID

(Core :: Networking: Cache, defect)

Description

[Nicholas Nethercote [inactive]]

ReportStorageMemory() in netwerk/cache2/CacheStorageService.cpp has the
following code:

    // Bypass memory-only entries, those will be reported when iterating
    // the memory only table. Memory-only entries are stored in both ALL_ENTRIES
    // and MEMORY_ONLY hashtables.
    nsRefPtr<mozilla::net::CacheEntry> const& entry = iter.Data();
    if (aTable->Type() == CacheEntryTable::MEMORY_ONLY ||
        entry->IsUsingDisk()) {
      size += entry->SizeOfIncludingThis(mallocSizeOf);
    }

erahm noticed (in bug 1189156 comment 13) that the comment doesn't match the code -- the comment suggests that MEMORY_ONLY entries should be skipped, but the
code measures such entries. I don't understand the second sentence in the
comment. But it seems like either the comment or the code should be changed.

Comment 1

[Nicholas Nethercote [inactive]]

Honza, you wrote this code in bug 964039. What do you think?

Comment 2

[Honza Bambas (:mayhemer)]

(In reply to Nicholas Nethercote [:njn] from comment #0)
> ReportStorageMemory() in netwerk/cache2/CacheStorageService.cpp has the
> following code:
> 
>     // Bypass memory-only entries, those will be reported when iterating
>     // the memory only table. Memory-only entries are stored in both
> ALL_ENTRIES
>     // and MEMORY_ONLY hashtables.
>     nsRefPtr<mozilla::net::CacheEntry> const& entry = iter.Data();
>     if (aTable->Type() == CacheEntryTable::MEMORY_ONLY ||
>         entry->IsUsingDisk()) {
>       size += entry->SizeOfIncludingThis(mallocSizeOf);
>     }
> 
> erahm noticed (in bug 1189156 comment 13) that the comment doesn't match the
> code -- the comment suggests that MEMORY_ONLY entries should be skipped, but
> the
> code measures such entries. I don't understand the second sentence in the
> comment. But it seems like either the comment or the code should be changed.

The code is correct.  We have two tables:

ALL_ENTRIES that keeps both disk and mem only
MEMORY_ONLY that keeps only the mem-only (the same already contained in ALL_)

the condition passes for either anything in MEMORY_ONLY or disk entries from the ALL_ one