Dmitriy, Alex, thanks for clarification, this is that i already mention in ticket. I think this approach correct. Try to provide PR soon.
thanks! > >------- Forwarded message ------- >From: "Alex Plehanov" < plehanov.a...@gmail.com > >To: dev@ignite.apache.org >Cc: arzamas...@mail.ru >Subject: Re: ignite PureJavaCrc32 vs java.util.zip.CRC32 bench. >Date: Tue, 21 Aug 2018 23:17:20 +0300 > >Dmitriy, yes, you correctly understood my idea. > >вт, 21 авг. 2018 г. в 22:03, Vladimir Ozerov < voze...@gridgain.com >: > >> Guys, >> >> Do we have any numbers for real workloads? Or at least Yardstick. >> >> вт, 21 авг. 2018 г. в 21:04, Dmitriy Pavlov < dpavlov....@gmail.com >: >> >> > Let us double-check current options. Maybe I'm mistaken about Alex >> > Plehanov's idea. >> > >> > Let's introduce the following variables and functions: >> > data - our block. >> > OldImpl(data) = X >> > NewImpl(data) = Y >> > NewImpl(data) ^ C = Y ^ C, where C is constant, ^ is bitwise XOR >> operation. >> > And NewImpl(data) ^ C = X = OldImpl(data) >> > >> > So new and old implementations give us X in all cases, we don't need >> any >> > compatibility mode. >> > >> > Let's try to adopt new fast implementation with C constant. >> > >> > Evgeniy, would you like to try? >> > >> > вт, 21 авг. 2018 г. в 15:51, Sergey Kozlov < skoz...@gridgain.com >: >> > >> > > Alex >> > > >> > > In that case the data becomes in unpredictable state (mix of new and >> old >> > > methods) and there's a chance that some partitions will be never >> touched >> > > and never converted. It will force us always support old compression. >> > > I suppose the explicit conversion that clearly say what is happening >> now >> > > and may be warn that the db files should backed up before version >> > upgrade. >> > > >> > > Also I think AI 3.0 will have set of incompatible changes that give >> us >> a >> > > chance to add an upgrade procedure where compression method update >> among >> > > other stuff will take into account. >> > > >> > > On Tue, Aug 21, 2018 at 11:58 AM, Alex Plehanov < >> plehanov.a...@gmail.com >> > > >> > > wrote: >> > > >> > > > Sergey, converting data also force us to introduce some flag to WAL >> > > > segments/records or convert WAL segments too. WAL segments can be >> > > archived, >> > > > they also can be compressed. >> > > > IMO we better should not introduce any compatibility modes, left >> data >> > as >> > > is >> > > > and always just convert crc value returned by zip.CRC32 to old >> format >> > > (xor >> > > > it) at runtime. >> > > > >> > > > 2018-08-21 0:12 GMT+03:00 Sergey Kozlov < skoz...@gridgain.com >: >> > > > >> > > > > Dmitriy >> > > > > >> > > > > Due to significant improvement and to reduce the number supported >> > > > > modes/options would be good to convert the data at the moment of >> > > upgrade. >> > > > > >> > > > > On Tue, Aug 21, 2018 at 12:03 AM, Dmitriy Setrakyan < >> > > > dsetrak...@apache.org >> > > > > > >> > > > > wrote: >> > > > > >> > > > > > Sergey, that was precisely my comment in the ticket: >> > > > > > >> > > > > > Can we add this option without breaking compatibility with >> previous >> > > > > > page/storage formats? If not, then this should support both >> > > > > implementation. >> > > > > > The default should be the new fastest implementation, but if we >> > > > encounter >> > > > > > the older, slower one, then we should print out a warning in >> the >> > log >> > > > and >> > > > > > automatically switch to the older implementation. >> > > > > > >> > > > > > On Mon, Aug 20, 2018 at 1:58 PM, Sergey Kozlov < >> > skoz...@gridgain.com >> > > > >> > > > > > wrote: >> > > > > > >> > > > > > > Hi Igniters >> > > > > > > >> > > > > > > I suppose that'll break compatibility for LFS (PDS). >> > > > > > > >> > > > > > > Do we plan to provide a migration guide w/o data loss for >> upgrade >> > > AI >> > > > > 2.x >> > > > > > to >> > > > > > > 3.0? >> > > > > > > >> > > > > > > On Mon, Aug 20, 2018 at 11:46 PM, Dmitriy Setrakyan < >> > > > > > dsetrak...@apache.org >> > > > > > > > >> > > > > > > wrote: >> > > > > > > >> > > > > > > > I commented in the ticket: https://issues.apache.org/ >> > > > > > > > jira/browse/IGNITE-9272 >> > > > > > > > >> > > > > > > > It if can integrate it correctly, according to my comment, >> in >> > 2.7 >> > > > > > > release, >> > > > > > > > it would be great. Otherwise, let's plan this change for >> 3.0 >> > > > release. >> > > > > > > > >> > > > > > > > D. >> > > > > > > > >> > > > > > > > On Mon, Aug 20, 2018 at 3:50 AM, Eduard Shangareev < >> > > > > > > > eduard.shangar...@gmail.com > wrote: >> > > > > > > > >> > > > > > > > > Hi, >> > > > > > > > > >> > > > > > > > > I have checked the benchmark and it shows great >> performance >> > > boost >> > > > > on >> > > > > > my >> > > > > > > > > laptop! >> > > > > > > > > >> > > > > > > > > +1 for this change. >> > > > > > > > > >> > > > > > > > > On Tue, Aug 14, 2018 at 9:01 PM Dmitriy Pavlov < >> > > > > > dpavlov....@gmail.com > >> > > > > > > > > wrote: >> > > > > > > > > >> > > > > > > > > > Hi Evgeniy, >> > > > > > > > > > >> > > > > > > > > > Thank you. I see that the ticket is unassigned. >> > > > > > > > > > >> > > > > > > > > > Would you like to contribute PR to be macro-benchmarked >> > with >> > > > > > Ignite? >> > > > > > > > > > >> > > > > > > > > > Sincerely, >> > > > > > > > > > Dmitriy Pavlov >> > > > > > > > > > >> > > > > > > > > > вт, 14 авг. 2018 г. в 20:57, Евгений Станиловский >> > > > > > > > > > < arzamas...@mail.ru.invalid >: >> > > > > > > > > > >> > > > > > > > > > > I fill the ticket, bench code attached there. >> > > > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-9272 >> > > > > > > > > > > Thanks! >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >Has anyone else run the benchmark and reproduced the >> > > > > performance >> > > > > > > > > > > >difference? >> > > > > > > > > > > > >> > > > > > > > > > > >On Tue, Aug 14, 2018 at 8:16 AM, Dmitriy Pavlov < >> > > > > > > > > dpavlov....@gmail.com >> > > > > > > > > > > >> > > > > > > > > > > >wrote: >> > > > > > > > > > > > >> > > > > > > > > > > >> It depends. >> > > > > > > > > > > >> >> > > > > > > > > > > >> CRC is a CPU-intensive operation, while WAL >> logging >> > and >> > > > page >> > > > > > > store >> > > > > > > > > > write >> > > > > > > > > > > >> are mostly about IO speed. >> > > > > > > > > > > >> >> > > > > > > > > > > >> In the same time, it can make the huge impact on >> > > machines >> > > > > with >> > > > > > > > fast >> > > > > > > > > IO >> > > > > > > > > > > >> and >> > > > > > > > > > > >> slow CPU. So if we can apply change proposed by >> > Evgeniy >> > > > and >> > > > > > > Alexey >> > > > > > > > > it >> > > > > > > > > > > >> could >> > > > > > > > > > > >> benefit performance because we save CPU. Later we >> can >> > > use >> > > > > it's >> > > > > > > > power >> > > > > > > > > > in >> > > > > > > > > > > a >> > > > > > > > > > > >> more efficient manner (e.g. with compression). >> > > > > > > > > > > >> >> > > > > > > > > > > >> вт, 14 авг. 2018 г. в 14:03, Yakov Zhdanov < >> > > > > > > yzhda...@apache.org >> > > > > > > > >: >> > > > > > > > > > > >> >> > > > > > > > > > > >> > Guys, what time in % does crc calculation take >> in >> > WAL >> > > > > > logging >> > > > > > > > > > process? >> > > > > > > > > > > >> > >> > > > > > > > > > > >> > --Yakov >> > > > > > > > > > > >> > >> > > > > > > > > > > >> > 2018-08-14 13:37 GMT+03:00 Dmitriy Pavlov < >> > > > > > > > dpavlov....@gmail.com >> > > > > > > > > > >: >> > > > > > > > > > > >> > >> > > > > > > > > > > >> > > Hi Alex, thank you for this idea. >> > > > > > > > > > > >> > > >> > > > > > > > > > > >> > > Evgeniy, Alex, would you like to submit the >> patch >> > > with >> > > > > > > > bypassing >> > > > > > > > > > > >> > > implementation differences to keep >> compatibility? >> > > > > > > > > > > >> > > >> > > > > > > > > > > >> > > Sincerely, >> > > > > > > > > > > >> > > Dmitriy Pavlov >> > > > > > > > > > > >> > > >> > > > > > > > > > > >> > > вт, 14 авг. 2018 г. в 12:06, Alex Plehanov < >> > > > > > > > > > > plehanov.a...@gmail.com >: >> > > > > > > > > > > >> > > >> > > > > > > > > > > >> > > > Hello, Igniters! >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > > > In java8 java.lang.zip.CRC32 methods become >> > > > intrinsic, >> > > > > > > > > moreover >> > > > > > > > > > > new >> > > > > > > > > > > >> > > > "update" method, which use ByteBuffer was >> > > > introduced. >> > > > > > > Since >> > > > > > > > we >> > > > > > > > > > > >> moved >> > > > > > > > > > > >> to >> > > > > > > > > > > >> > > > java8, perhaps we really can get performance >> > boost >> > > > by >> > > > > > > using >> > > > > > > > > > > >> standard >> > > > > > > > > > > >> > > > java.lang.zip.CRC32 instead of >> PureJavaCrc32. >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > > > About compatibility: looks like >> PureJavaCrc32 >> > > > > implements >> > > > > > > the >> > > > > > > > > > same >> > > > > > > > > > > >> > > algorithm >> > > > > > > > > > > >> > > > as java.lang.zip.CRC32. These two >> > implementations >> > > > uses >> > > > > > the >> > > > > > > > > same >> > > > > > > > > > > >> > > polynomial >> > > > > > > > > > > >> > > > and the same initial value. The only >> difference >> > is >> > > > > final >> > > > > > > xor >> > > > > > > > > > mask >> > > > > > > > > > > >> > > > (0xFFFFFFFF for java.lang.zip.CRC32). So, we >> can >> > > > > easily >> > > > > > > > > convert >> > > > > > > > > > > >> from >> > > > > > > > > > > >> > > > PureJavaCrc32 >> > > > > > > > > > > >> > > > to standard CRC32 and vice versa, using this >> > > > > expression: >> > > > > > > > crc32 >> > > > > > > > > > ^= >> > > > > > > > > > > >> > > > 0xFFFFFFFF >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > > > 2018-08-14 0:19 GMT+03:00 Eduard Shangareev >> < >> > > > > > > > > > > >> > eduard.shangar...@gmail.com >> > > > > > > > > > > >> > > >: >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > > > > Evgeniy, >> > > > > > > > > > > >> > > > > >> > > > > > > > > > > >> > > > > Could you share benchmark code? And please >> > share >> > > > > what >> > > > > > > > > version >> > > > > > > > > > of >> > > > > > > > > > > >> JVM >> > > > > > > > > > > >> > > > > you have used. >> > > > > > > > > > > >> > > > > >> > > > > > > > > > > >> > > > > On Mon, Aug 13, 2018 at 10:44 PM Zhenya >> > > > > > > > > > > >> < arzamas...@mail.ru.invalid >> > > > > > > > > > > >> > >> > > > > > > > > > > >> > > > > wrote: >> > > > > > > > > > > >> > > > > >> > > > > > > > > > > >> > > > > > I think it would break backward >> > compatibility, >> > > > as >> > > > > > > > Nikolay >> > > > > > > > > > > >> mentioned >> > > > > > > > > > > >> > > > above >> > > > > > > > > > > >> > > > > > we would take exception here: >> > > > > > > > > > > >> > > > > > >> > > > > > > > > > > >> > > > > > [1] >> > > > > > > > > > > >> > > > > > >> > > > > > > > > > > >> > > > > > https://github.com/apache/ >> > > > > > > ignite/blob/master/modules/ >> > > > > > > > > > > >> > > > > core/src/main/java/org/apache/ >> > > > > > > ignite/internal/processors/ >> > > > > > > > > > > >> > > > > >> cache/persistence/file/FilePageStore.java#L372 >> > > > > > > > > > > >> > > > > > >> > > > > > > > > > > >> > > > > > thats why i question for community >> thoughts >> > > > here. >> > > > > > > > > > > >> > > > > > >> > > > > > > > > > > >> > > > > > > Hi Evgeniy, >> > > > > > > > > > > >> > > > > > > >> > > > > > > > > > > >> > > > > > > would you like to submit a patch with >> > CRC32 >> > > > > > > > > implementation >> > > > > > > > > > > >> > change? >> > > > > > > > > > > >> > > > > > > >> > > > > > > > > > > >> > > > > > > Sincerely, >> > > > > > > > > > > >> > > > > > > Dmitriy Pavlov >> > > > > > > > > > > >> > > > > > > >> > > > > > > > > > > >> > > > > > > пн, 13 авг. 2018 г. в 22:08, Евгений >> > > > > Станиловский >> > > > > > > > > > > >> > > > > > > < arzamas...@mail.ru.invalid >: >> > > > > > > > > > > >> > > > > > > >> > > > > > > > > > > >> > > > > > >> Hi, igniters, i wrote a simple bench, >> > looks >> > > > > like >> > > > > > > > > > > >> PureJavaCrc32 >> > > > > > > > > > > >> > has >> > > > > > > > > > > >> > > > > > >> performance problems in compatible >> with >> > > > > > zip.CRC32. >> > > > > > > > > > > >> > > > > > >> >> > > > > > > > > > > >> > > > > > >> Benchmark Mode Cnt Score Error Units >> > > > > > > > > > > >> > > > > > >> BenchmarkCRC.Crc32 avgt 5 >> 1088914.540 ± >> > > > > > 368851.822 >> > > > > > > > > ns/op >> > > > > > > > > > > >> > > > > > >> BenchmarkCRC.pureJavaCrc32 avgt 5 >> > > > 6619408.049 ± >> > > > > > > > > > 3746712.210 >> > > > > > > > > > > >> > ns/op >> > > > > > > > > > > >> > > > > > >> >> > > > > > > > > > > >> > > > > > >> thoughts? >> > > > > > > > > > > >> > > > > > >> > > > > > > > > > > >> > > > > >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > -- >> > > > > > > > > > > Евгений Станиловский >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > -- >> > > > > > > Sergey Kozlov >> > > > > > > GridGain Systems >> > > > > > > www.gridgain.com >> > > > > > > >> > > > > > >> > > > > >> > > > > >> > > > > >> > > > > -- >> > > > > Sergey Kozlov >> > > > > GridGain Systems >> > > > > www.gridgain.com >> > > > > >> > > > >> > > >> > > >> > > >> > > -- >> > > Sergey Kozlov >> > > GridGain Systems >> > > www.gridgain.com >> > > >> > -- Zhenya Stanilovsky
