> On Feb 22, 2018, at 8:14 PM, Seán Coffey <[email protected]> wrote: > > This looks good to me Max. I think it'll solve the issue reported. > > line 109 : I guess this could turn negative if timeLimit was value < 60. I > don't think that's possible unless we're dealing with a strange config!
Negative time might still work. In fact, time() values before the epoch are also comparable, unless less than -2^32. > > For the code comment, may a minor edit : > > 111 // Only trigger a cleanup when very old entries exist > 112 // (lifespan + 1 min ago). This ensures a cleanup is done > 113 // at most every minute. Accepted. Thanks Max > > Regards, > Sean. > > On 22/02/18 08:36, 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 >> >
