Hi all, Yakov pointed me that I didn't remove Reenrtrant Lock after it use. Data Structures' are stored in HashMap, so serialization and deserialization time depends on HashMap size and becomes very expensive without removing unused data structures.
I added removing for unused Reenrtrant Locks and run benchmark again. Now difference isn't so dramatic as it was : ~1000 op/s vs ~4500 op/s Also I decided to test it with some number of pre-created Data Structures in cache: 100 reentrantlock in cache: ~700 op/s vs ~4500 op/s 1000 reentrantlock in cache: ~180 op/s vs ~4500 op/s 10000 reentrantlock in cache: ~25 op/s vs ~ 4500 op/s I've run TC: http://ci.ignite.apache.org/viewLog.html?buildId=640339&tab=buildResultsDiv&buildTypeId=Ignite20Tests_RunAll it doesn't look very "green", 4 suites failed with timeout, I've re-run them: http://ci.ignite.apache.org/viewLog.html?buildId=645487&; - green http://ci.ignite.apache.org/viewLog.html?buildId=645488&; - green http://ci.ignite.apache.org/viewLog.html?buildId=645485&tab=buildResultsDiv&buildTypeId=Ignite20Tests_IgniteCacheFailover2 - 1 failed test, I'm checking it. http://ci.ignite.apache.org/viewType.html?buildTypeId=Ignite20Tests_IgniteDataGridRestarts&branch_Ignite20Tests=pull%2F2053%2Fhead&tab=buildTypeStatusDiv - still running also there're several red tests, but almost all of them are flaky, except 2 test - I'm checking them too. Thanks, Mikhail. On Fri, Jun 2, 2017 at 7:00 PM, Mikhail Cherkasov <mcherka...@gridgain.com> wrote: > https://github.com/apache/ignite/pull/2053/files > > I'm checking TC, now, I'll send it for review as soon as pull request > passes TC. > > On Fri, Jun 2, 2017 at 6:41 PM, Dmitriy Setrakyan <dsetrak...@apache.org> > wrote: > >> Wow, what was patched? Can you provide the code sample being benchmarked? >> >> On Fri, Jun 2, 2017 at 8:00 AM, Mikhail Cherkasov < >> mcherka...@gridgain.com> >> wrote: >> >> > Looks like dev list doesn't allow to include pictures in emails, so >> there >> > are links to plots: >> > >> > Throughput plot without any changes: >> > https://www.dropbox.com/s/mc3v5m49u4i2be3/no-changes.png?dl=0 >> > >> > the same plot with patched ignite: >> > https://www.dropbox.com/s/cyojw9nz4smew87/with-chages.png?dl=0 >> > >> > 17000 operations/sec vs 70 operations/sec. >> > >> > On Fri, Jun 2, 2017 at 4:57 PM, Mikhail Cherkasov < >> mcherka...@gridgain.com >> > > wrote: >> > >> >> Hi all, >> >> >> >> I prepared a benchmark for ignite reentrant lock, the benchmark updates >> >> cache values under the reentrant lock. >> >> The benchmark is based on s real case, when user can't use regular >> cache >> >> locks, because they >> >> prevent partition map exchange and as result don't allow new nodes join >> >> cluster. >> >> >> >> Throughput plot without any changes: >> >> >> >> [image: Inline image 1] >> >> >> >> the same plot with patched ignite: >> >> >> >> [image: Inline image 2] >> >> >> >> >> >> On Thu, Jun 1, 2017 at 1:29 AM, Dmitriy Setrakyan < >> dsetrak...@apache.org> >> >> wrote: >> >> >> >>> Won't it be confusing from a user stand point to have multiple data >> >>> structures with the same name? Also, what is the performance impact of >> >>> this >> >>> change? >> >>> >> >>> D. >> >>> >> >>> On Wed, May 31, 2017 at 8:23 AM, Semyon Boikov <sboi...@gridgain.com> >> >>> wrote: >> >>> >> >>> > Hi Mikhail, >> >>> > >> >>> > As far as I remember for some reason we wanted to guarantee that all >> >>> data >> >>> > structures have unique names. But now I don't see why this can be >> >>> needed >> >>> > and it seems we do not need this data structures map at all, if >> nobody >> >>> have >> >>> > objection I think you can implement suggested change. >> >>> > >> >>> > Thanks! >> >>> > >> >>> > On Wed, May 31, 2017 at 3:04 PM, Mikhail Cherkasov < >> >>> > mcherka...@gridgain.com> >> >>> > wrote: >> >>> > >> >>> > > Hi all, >> >>> > > >> >>> > > All DataStructures are stored in one Map which itself is stored in >> >>> > > utilityCache, this makes high contention on DS creation or >> removing, >> >>> it >> >>> > > requires to acquire Map's lock and manipulation with the Map under >> >>> the >> >>> > > lock. So all threads in cluster should wait for this lock to >> create >> >>> or >> >>> > > remove DS. >> >>> > > >> >>> > > I don't see any reason to store all DS in one map, we already >> have >> >>> > > utilityCache and can save DSs directly in utilityCache, to >> >>> distinguish DS >> >>> > > with other objects in utilityCache I use composite key, the first >> >>> part of >> >>> > > which is DATA_STRUCTURES_KEY, second one is DS's name, also DS >> type >> >>> can >> >>> > be >> >>> > > added, this will allow us to create different DS with the same >> name. >> >>> > > >> >>> > > There is draft implementations, all DSs are stored with unique >> key in >> >>> > > utilityCache: >> >>> > > https://github.com/apache/ignite/pull/2046/files >> >>> > > >> >>> > > May be there's some reason to store all DS in one Map that I >> missed? >> >>> > > Any thoughts? >> >>> > > >> >>> > > >> >>> > > -- >> >>> > > Thanks, >> >>> > > Mikhail. >> >>> > > >> >>> > >> >>> >> >> >> >> >> >> >> >> -- >> >> Thanks, >> >> Mikhail. >> >> >> > >> > >> > >> > -- >> > Thanks, >> > Mikhail. >> > >> > > > > -- > Thanks, > Mikhail. > -- Thanks, Mikhail.
