1147973 - Detailed disk cache hit rate telemetry is wrong

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Michal Novotny [:michal]]

Attachment: fix

Entries that are created as new are not reported.

Comment 1

[Honza Bambas (:mayhemer)]

Comment on attachment 8583918 [details] [diff] [review]
fix

Review of attachment 8583918 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/cache2/CacheEntry.cpp
@@ +386,5 @@
>      mozilla::MutexAutoUnlock unlock(mLock);
>  
> +    if (reportMiss) {
> +      CacheFileUtils::DetailedCacheHitTelemetry::AddRecord(
> +        CacheFileUtils::DetailedCacheHitTelemetry::MISS, TimeStamp());

You are actually filling the gap when we open the file directly (not getting OnFileReady call), right?

Why exactly you don't want to report NETWORK_CACHE_V2_MISS_TIME_MS here?  I think to report 0ms is pretty much OK, we want to see how fast we detect a cache miss, no?

Comment 2

[Michal Novotny [:michal]]

Attachment: patch v2

(In reply to Honza Bambas (:mayhemer) from comment #1)
> You are actually filling the gap when we open the file directly (not getting
> OnFileReady call), right?

Right.

 
> Why exactly you don't want to report NETWORK_CACHE_V2_MISS_TIME_MS here?  I
> think to report 0ms is pretty much OK, we want to see how fast we detect a
> cache miss, no?

I wanted to fix hit rate stats without affecting NETWORK_CACHE_V2_MISS_TIME_MS but I'm not against.

Comment 3

[Michal Novotny [:michal]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9bf6a6c84045

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9bf6a6c84045