add an option to pass a weight to put and replace methods for weights based on the byte size of a value

[GoogleCodeExporter]

Current Behavior:
weights computed via a weighter object

Desired Behavior:
keep current behavior but add an option to pass a weight to put and replace 
methods for weights based on the byte size of the value. This helps performance 
as low level DBes like BerkeleyDB return byte arrays anyway so just passing the 
length field of the existing byte array to the put and replace methods is 
better than having a weighter object that has to use cpu cycles to calculate 
the size (via the creation of another byte array or by other methods).

Complexity / Amount of Change Required:
this sort of thing should do the job>

 @Override
  public V put(K key, V value) {
    return put(key, value, false,-1);
  }

  @Override
  public V putIfAbsent(K key, V value) {
    return put(key, value, true,-1);
  }

  V put(K key, V value, boolean onlyIfAbsent) {
     return  put(key,value,onlyIfAbsent,-1);
  }

  /**
   * Adds a node to the list and the data store. If an existing node is found,
   * then its value is updated if allowed.
   *
   * @param key key with which the specified value is to be associated
   * @param value value to be associated with the specified key
   * @param onlyIfAbsent a write is performed only if the key is not already
   *     associated with a value
   * @return the prior value in the data store or null if no mapping was found
   */
  V put(K key, V value, boolean onlyIfAbsent, long theWeight) {
    checkNotNull(key, "null key");
    checkNotNull(value, "null value");

//changed from int to long to support max capacity based on size that is not 
limited to 2GB
    final long weight = theWeight < 1 ? weigher.weightOf(value) : theWeight;
    final WeightedValue<V> weightedValue = new WeightedValue<V>(value, weight);
    final Node node = new Node(key, weightedValue);

//and same for replace methods

Original issue reported on code.google.com by pariodeu...@googlemail.com on 20 Apr 2011 at 1:24

[GoogleCodeExporter]

If the value is a byte array, then Weighers#byteArray() can be used to specify 
the weight as the length value.

If the value is deserialized and the desire is to use its byte[] equivalent as 
the weight, then the simplest work around is to use a decorator. A custom 
weigher could capture the weight by calling a #size() method. This would have 
marginal overhead.

There are, of course, hacky ways to avoid that overhead. An example might be to 
use a ThreadLocal to pass the the length through to the weigher. That's not to 
say that I'd ever stomach the idea of doing this.

The additional methods would be easy, I agree. Before venturing down that path 
I'd like to know if either of the first two approaches would be acceptable.

Original comment by Ben.Manes@gmail.com on 20 Apr 2011 at 2:01

[GoogleCodeExporter]

Using a decorator is certainly an option but I think that adding the additional 
methods is worth the small effort and considering that this is meant to be a an 
object cache I think others would appreciate the simplicity of having the 
option of passing in a weight without having to bother with creating custom 
weighter's.

Original comment by pariodeu...@googlemail.com on 20 Apr 2011 at 10:47

[GoogleCodeExporter]

I'm still playing with the idea, only because public API creep needs more due 
diligence. I agree that its a reasonable request and non-invasive, so I'm prone 
to making the change.

What are your thoughts on the following failure case that this would allow?

- Client constructs the map with the default weigher (value --> 1)
- All calls use new weighted method, so default weigher is never used
- New code (e.g. 1yr later) adds a feature and uses the regular Map APIs. This 
lets some cached items to be incorrectly weighed.
- Application runs smoothly and tests don't detect the issue. It eventually 
fails in production. It can only be reproduced there due to the load / uptime 
requirements.

Is this scenario the library partially at fault for enabling innocent mistakes?

Original comment by Ben.Manes@gmail.com on 20 Apr 2011 at 9:37

[GoogleCodeExporter]

Sorry, poorly edited the last line. :)
-
In this scenario, is the library partially at fault for enabling innocent 
mistakes?

Original comment by Ben.Manes@gmail.com on 20 Apr 2011 at 9:38

[GoogleCodeExporter]

Perhaps you could avoid this issue with the use of simple boolean flags that 
indicate if the default weighter was used or not.

An alternative would be for the client to specify with an additional optional 
builder method whether they want to use the weighter methods or the 
default/specified weighter.
A call to the weighter methods when the user has not chosen to enable them 
could return an exception/null/false or whatever and vice versa.

Original comment by pariodeu...@googlemail.com on 21 Apr 2011 at 1:09

[GoogleCodeExporter]

Some discussion on the group w.r.t. to this issue.

http://groups.google.com/group/concurrentlinkedhashmap/browse_thread/thread/a7ef
2ef3f4d63171

Original comment by Ben.Manes@gmail.com on 5 May 2011 at 4:01

• Changed state: Accepted

[GoogleCodeExporter]

Can you respond to the last message in the group thread? I've finally getting 
back to CLHM and am hoping to release this week.

Message copied below:
-- 
If you know the weight up-front for both the key and value, what is 
the harm of using a wrapper and a custom weigher? The only issue I see 
is that it requires an extra reference and int field, which is minor 
memory-wise. This would allow you to work with the current APIs, 
rather than needing new ones be added. 

That was satisfactory before, even though not your preference. Is that 
no longer the case?

Original comment by Ben.Manes@gmail.com on 17 May 2011 at 7:57

[GoogleCodeExporter]

While not perfect, at the moment I prefer the value decorator as a work around.

Please reopen if you disagree. I am happy to reconsider.

I plan on releasing tomorrow unless you speak up.

Thanks!
Ben

Original comment by Ben.Manes@gmail.com on 20 May 2011 at 12:05

• Changed state: WontFix