Hello Igniters! So we have merged the patch. If there are further steps with CLHM, feel free to contribute.
Regards, -- Ilya Kasnacheev 2018-08-10 17:50 GMT+03:00 Andrey Gura <ag...@apache.org>: > Stas, > > SkipList implementation offers O(log n) for get/put/contains > operations while CLHM - O(1). So it is suitable for small data sets > but will have serious performance impact for the big ones. > > However, it seems it's time for right choice: correctness or > performance. The answer seems obvious ) > On Tue, Jul 24, 2018 at 4:45 PM Stanislav Lukyanov > <stanlukya...@gmail.com> wrote: > > > > It seems that we currently use the CLHM primarily as a FIFO cache. > > I see two ways around that. > > > > First, we could implement such LRU/FIFO cache based on another, properly > supported data structure from j.u.c. > > ConcurrentSkipListMap seems OK. I have a draft implementation of > LruEvictionPolicy based on it that passes functional tests, > > but I haven’t benchmarked it yet. > > > > Second, Guava has a Cache API with a lot of configuration options that > we could use (license is Apache, should be OK). > > > > Stan > > > > From: Nikolay Izhikov > > Sent: 24 июля 2018 г. 16:27 > > To: dev@ignite.apache.org > > Subject: Re: ConcurrentLinkedHashMap works incorrectly after clear() > > > > Hello, Ilya. > > > > May be we need to proceed with ticket [1] "Get rid of org.jsr166. > ConcurrentLinkedHashMap"? > > > > Especially, if this class is broken and buggy. > > > > [1] https://issues.apache.org/jira/browse/IGNITE-7516 > > > > В Вт, 24/07/2018 в 16:20 +0300, Ilya Lantukh пишет: > > > Thanks for revealing this issue! > > > > > > I don't understand why should we disallow calling clear(). > > > > > > One way how it can be re-implemented is: > > > 1. acquire write locks on all segments; > > > 2. clear them; > > > 3. reset size to 0; > > > 4. release locks. > > > > > > Another approach is to calculate inside > > > ConcurrentLinkedHashMap.Segment.clear() how many entries you actually > > > deleted and then call size.addAndGet(...). > > > > > > In both cases you'll have to replace LongAdder with AtomicLong. > > > > > > On Tue, Jul 24, 2018 at 4:03 PM, Ilya Kasnacheev < > ilya.kasnach...@gmail.com> > > > wrote: > > > > > > > Hello igniters! > > > > > > > > So I was working on a fix for > > > > https://issues.apache.org/jira/browse/IGNITE-9056 > > > > The reason for test flakiness turned out our ConcurrentLinkedHashMap > (and > > > > its tautological cousin GridBoundedConcurrentLinkedHashMap) is > broken :( > > > > > > > > When you do clear(). its size counter is not updated. So sizex() will > > > > return the old size after clear, and if there's maxCnt set, after > several > > > > clear()s it will immediately evict entries after they are inserted, > > > > maintaining map size at 0. > > > > > > > > This is scary since indexing internals make intense use of > > > > ConcurrentLinkedHashMaps. > > > > > > > > My suggestion for this fix is to avoid ever calling clear(), making > it > > > > throw UnsupportedOperationException and recreating/replacing map > instead of > > > > clear()ing it. Unless somebody is going to stand up and fix > > > > ConcurrentLinkedHashMap.clear() properly. Frankly speaking I'm > afraid of > > > > touching this code in any non-trivial way. > > > > > > > > -- > > > > Ilya Kasnacheev > > > > > > > > > > > > > > > >
