songchuanyuan66 / concurrentlinkedhashmap

Automatically exported from code.google.com/p/concurrentlinkedhashmap
Apache License 2.0
0 stars 1 forks source link

(policy, maximumCapacity, concurrencyLevel) constructor on CLHM1 doesn't set concurrencyLevel #7

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Summary says it all. And big thanks for your work!

Original issue reported on code.google.com by earwin@gmail.com on 17 May 2009 at 1:45

GoogleCodeExporter commented 9 years ago
Great find!

I have deprecated CLHM v1 and have fixed it in v2.

Original comment by Ben.Manes@gmail.com on 19 May 2009 at 10:12

GoogleCodeExporter commented 9 years ago
Does that mean v2 is now reliable?

Original comment by earwin@gmail.com on 19 May 2009 at 10:23

GoogleCodeExporter commented 9 years ago
It will be reliable by the end of the week, but isn't yet. I have a race 
condition 
on the removal which I need to debug and hope to fix tonight.

I should have branched off before I checked in so v1 was still available until 
I can 
give the thumbs up. It still is, via history, but I should have been better 
about 
that... sorry.

Original comment by Ben.Manes@gmail.com on 19 May 2009 at 5:08

GoogleCodeExporter commented 9 years ago
By the way, will you consider static factory methods a-la GoogleCollections for 
CLHM?
ConcurrentLinkedHashMap<SomeKeyType, AndValueType> qwe = new
ConcurrentLinkedHashMap<SomeKeyType, AndValueType>(LRU, 5, 5);
loses big way to
ConcurrentLinkedHashMap<SomeKeyType, AndValueType> qwe =
newConcurrentLinkedHashMap(LRU, 5, 5);

Original comment by earwin@gmail.com on 19 May 2009 at 5:17

GoogleCodeExporter commented 9 years ago
I agree - its verbose. I played with that, but it felt messy to put directly on 
the 
class. I think it would be better to, like Google Collections, do that by a 
separate 
utility in a factory. I have a fairly substantial extension to GC to add more 
utilities / data-structures, so for example I have Maps2#newIndexMap(...). 
(Many 
keys->value, changes through a key affect all keys).

The other issue is that - how often will you write that code? In my caching 
framework its once. All clients specify via a Spring extension the 
configuration and 
inject the "Cache" facade. There code is unaware of how its configured, so it 
can 
easily be changed from "fixed" (CLHM) to "ehcache" to "nullcache" via 
configuration. 
That covers 95% of the cases.

The other 5% that I've seen are done by perf analysis. For example, 
Class#newInstance
(String className) can become expensive if called regularly for a class that 
doesn't 
exist (scans the jars). This can be solved by having a utility which contains 
an 
internal cache storing whether it the class previously found. That way, it can 
no-op 
quickly (Map<String, Boolean>). These cases are rare enough that the verbosity 
didn't seem too bad.

Do you expect to see yourself manually creating a CLHM regularly? If so, is it 
done 
in a way that doesn't feel like a bad practice (e.g. leaking cache config in 
scenario #1)? I can reconsider adding the static factories if there are good 
use-
cases.

Original comment by Ben.Manes@gmail.com on 19 May 2009 at 5:47

GoogleCodeExporter commented 9 years ago
>  like Google Collections, do that by a separate utility in a factory
In fact they switched to static constructors named 'create' in all their 
publicly
visible collection implementations. Take ArrayListMultimap for instance. I bet 
they'd
did it to jdk collections, if they could.

> Do you expect to see yourself manually creating a CLHM regularly?
Well, okay, I do it twice. Just wanted all of my code to look consistent. 
There's
probably nothing bad in making static constructor for that purporse myself :)

> If so, is it done in a way that doesn't feel like a bad practice (e.g. leaking
cache config in 
scenario #1)?
CLHM is an implementation detail that doesn't need configuration. Just like your
usage of CHM inside CLHM.

I don't really insist, just wondered if you like the idea.

Original comment by earwin@gmail.com on 20 May 2009 at 6:40

GoogleCodeExporter commented 9 years ago
Ahh, I haven't updated to v1.0.  I have code that depends on ReferenceMap 
accepting 
a backing data store to support notifying listeners. This isn't compatible with 
their rewrite, so I've held off on the upgrade...

I'll reconsider the idea, ahd may adopt it. Its worth giving it a go and seeing 
how 
the code looks. No promises! :)

Original comment by Ben.Manes@gmail.com on 20 May 2009 at 6:46