2

The Javadoc says:

Returns a view of the entries stored in this cache as a thread-safe map. Modifications made to the map directly affect the cache.

What I'm missing is the information about whether access to the view influences the admission and eviction policies. According to this old related issue it does not:

In Guava's CacheBuilder we added the asMap() view specifically to allow bypassing the cache management routines. There a cache.asMap().get(key) is a peek operation.

This surely makes sense. OTOH the view provides many operations unavailable directly and users may be tempted to use them hoping that they update the access statistics just like direct operations.

For example, I found myself using cache.asMap().putIfAbsent as my values are functions of the keys, so replacing them is pointless. I'd like it to work exactly as cache.put in case the entry was absent.

maaartinus
  • 44,714
  • 32
  • 161
  • 320
  • At some point the intent was lost and Guava shifted so that all of the routines are called, except `CacheStats` is not recorded on a `Map.get`. Caffeine emulates that for compatibility, though I don't quite understand the intent anymore. All other operations are as if the operation was on the Cache interface, but separated for a more idiomatic API. – Ben Manes Jan 07 '18 at 18:02
  • @BenManes I'm sure, having a view behaving *exactly* as the cache is very useful, both for the additional operations and for passing it to a third party map-expecting code. I guess, having a peeking view may be sometimes useful, too and so may be Guava compatibility. I'm afraid, providing multiple view wouldn't carry its weight. Neither would probably letting the user choose the behavior of `asMap`. +++ So maybe just document the current one? Maybe in a way allowing both Guava compatibility and full emulation - maybe something like "it's unspecified if `CacheStats` is recorded on a `Map.get`"? – maaartinus Jan 07 '18 at 18:43
  • I think we decided to not record stats on any `Map` methods, as stated in Guava's `CacheStats`. The intent might have been because internal users were surprised when `Map` had side-effects, which is why `MapMaker's` computing `get` was awkward. But this is such a minor detail it was likely forgotten and not always honored, and also seems wrong in retrospect. So we probably made mistakes but not in ways that have mattered to users most of the time. – Ben Manes Jan 07 '18 at 18:53

1 Answers1

3

This was part of the original discussion of Guava's Map.get, but was likely a poor idea, miscommunicated, and eventually lost. A rational was due to users not expecting side effects for most Map operations, which MapMaker changed with computing maps and thus broke the equals method.

In retrospect treating any Map methods as different breaks the principle of least astonishment and is not very useful. This was likely realized during the implementation, but due to the disjoint team and abundance of details, an aspect that I forgot. We also had decided on the principle that users shouldn't need to know how the policies work or have configuration knobs to influence their implementation, which a quiet get would have exposed.

However, one aspect did remain for better or worse. Unlike Cache.getIfPresent, Map.get will not record hit rate statistics. Similarly for all other Map operations, they may be opted out of updating CacheStats. In Guava this is stated as,

No stats are modified by operations invoked on the asMap view of the cache.

This is slightly modified in Caffeine for additional Java 8 methods,

No stats are modified by non-computing operations invoked on the 
asMap view of the cache.

Likely this opt-out of statistics should not have occurred, but is the remanent of that original discussion. This is a subtle detail that it may not be honored in full, as I believe Guava's addition of the computing Map methods do not. Thankfully it is a minor enough detail to have not caused many issues and could be changed if deemed worthwhile.

Ben Manes
  • 9,178
  • 3
  • 35
  • 39
  • So everything works the same, except for the `CacheStats`. Would you add such a sentence to `asMap()` as when using it, the user doesn't think about the stats? – maaartinus Jan 07 '18 at 19:48
  • Perhaps we should. I don't know if it is better or not to, in case we decide its a misfeature and silently change it hoping that most users won't realize... – Ben Manes Jan 07 '18 at 19:58
  • For me, the important part is the non-peeking behavior (an entry gets sort of bumped to the top, even when accessed via the view). I'd document the stats, too, with a notice that it may change (actually, it's already documented, just in a different class). But that's your choice, I've got my answer. Thanks again for caffeine! – maaartinus Jan 07 '18 at 20:10
  • 1
    Perhaps the reason is because it gets weird in a Map. Does `put` and `putIfAbsent` increase the hit count if the value is present? What does `replace` do? Does iterating count as a hit? Dropping stats for consistency so that all methods are treated uniformly makes sense as the easiest answer. – Ben Manes Jan 09 '18 at 00:26