Hi Max,
85 // has larger timestamp.
86 entries.addFirst(t);
shouldn't you set oldestTime here as well?
best regards,
-- daniel
On 23/02/2018 15:21, Xuelei Fan wrote:
Looks fine to me.
Xuelei
On 2/23/2018 6:13 AM, Weijun Wang wrote:
Updated at http://cr.openjdk.java.net/~weijun/8197518/webrev.02/.
1. ConcurrentHashMap used in MemoryCache. No more
"content.remove(key)" but it's actually useless because rc.isEmpty()
will not be true normally. I'll think about how to clean up unused
AuthList objects in another RFE.
2. AuthList now removes entries from tail backwards until one is
within lifespan. I still kept an oldestTime field. In my experiment it
is useful but a bigger value does not help much, so I just hardcoded
it to 5 seconds.
Thanks
Max
On Feb 23, 2018, at 4:57 PM, Weijun Wang <weijun.w...@oracle.com> wrote:
Brilliant idea. I will do more experiment.
Thanks
Max
On Feb 23, 2018, at 11:25 AM, Xuelei Fan <xuelei....@oracle.com> wrote:
Hi Weijun,
I thought more about the performance impact. The impact may mainly
come from the big size of the cached entries.
The current implementation needs to go over the full list: find the
1st expired item and then remove the rest. The previous one have an
additional round with entries.indexOf(). Could we just start from
the end of the list?
while (true) {
E e = entries.removeLast();
if e not expired {
add it back and break;
}
};
If the list is really big, and the lifetime is significant big as
well (>> 1 minute), iterate from the oldest item (backward from the
end of the list) may be much more effective. LinkedList itself is
not synchronized, so as if there is not too much items to go over,
the performance should be fine. I'm hesitate to hard code a cleanup
every 1 minute strategy. If we clean often, there may be not too
much items to go over in the list. So we might be able to remove
the "at most every minute" strategy.
Xuelei
On 2/22/2018 5:55 PM, Weijun Wang wrote:
Updated webrev at
http://cr.openjdk.java.net/~weijun/8197518/webrev.01/.
On Feb 23, 2018, at 9:02 AM, Weijun Wang <weijun.w...@oracle.com>
wrote:
You mean I can save it somewhere and only update it when a cleanup
is performed?
This should be better. Now there will be only isEmpty(),
getFirst() and addFirst(), and one less getLast().
Thanks
Max
On Feb 23, 2018, at 1:45 AM, Xuelei Fan <xuelei....@oracle.com>
wrote:
Looks like list synchronization is a factor of the performance
impact. Maybe, you can have a private time for the oldest entry
so don't access/iterate/cleanup entries list until necessary.
The "at most every minute" may be not a good strategy in some
situations.
In fact, it's now almost "exactly every minute". What situations do
you think it's not good? I cannot use size() because I have to
remember all entries with lifespan to be correct.
Thanks
Max
Xuelei
On 2/22/2018 12:36 AM, Weijun Wang wrote:
Please take a review at
http://cr.openjdk.java.net/~weijun/8197518/webrev.00/
Two notes:
1. I tried list.subList(here, end).clear() but it's not faster.
2. I have looked at ConcurrentHashMap + ConcurrentSkipListMap
but will need more time to verify its correctness and measure
the performance gain. Since the bug is reported on 8u, a safer
fix looks better.
Noreg-perf.
Thanks
Max
