On Wed, Aug 9, 2017 at 4:59 AM, Николай Ижиков <nizhikov....@gmail.com> wrote:
> Vladimir, > > > There is nothing wrong with U.currentTimeMillis() at the > moment. > > I think we can't rely on the return value for time measurement. > Is it true? Is it OK for you? > > It very counterintuitive for me as newcomer. > I agree with Vladimir. There is nothing broken, yet we are trying to fix something. However, we are definitely running a risk of breaking something, if we "fix" it. I would leave this method alone. Nikolai, if you believe that the method is not reliable, write a test that will demonstrate it. > > 2017-08-09 14:55 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>: > > > You cannot check it with benchmarks, because behavior of this method will > > vary between different JVMs, OSes and hardware. It can be different even > > with the same OS depending on it's settings. Again - let's just avoid > > unnecessary work. There is nothing wrong with U.currentTimeMillis() at > the > > moment. > > > > On Wed, Aug 9, 2017 at 2:52 PM, Dmitry Pavlov <dpavlov....@gmail.com> > > wrote: > > > > > Vladimir, could we check it using benchmarks? Internet contains a lot > of > > > articles about this issue. But do we know if it is still actual for new > > > VMs? > > > > > > ср, 9 авг. 2017 г. в 14:50, Dmitry Pavlov <dpavlov....@gmail.com>: > > > > > > > It seems System.currentTimeMillis () is now in intrinsic list. This > > means > > > > on modern JVMs performance penalty will not be so significiant. > > > > > > > > Nickolay, could you please raise standalone ticket for > > > U.currentTimeMillis > > > > () ? > > > > > > > > Could you also please check if system.nanoTime / system.currentTimeMs > > can > > > > fix https://issues.apache.org/jira/browse/IGNITE-5963 When you > create > > a > > > > PR, I can start several run for Ignite Cache 6 suite to check if > issue > > is > > > > still reprodacible. > > > > > > > > ср, 9 авг. 2017 г. в 14:41, Yakov Zhdanov <yzhda...@apache.org>: > > > > > > > >> Nickolay, IgniteUtils#currentTimeMillis() is some kind of an old > > > heritage. > > > >> I guess nobody remembers when this method has been introduced. I > agree > > > >> that > > > >> we can use System.currentTimeMillis(). I would suggest you file a > > ticket > > > >> and replace this method calls with System.currentTimeMillis(). > Sounds > > > >> good? > > > >> > > > >> As far as reliable elapsed time measurement I agree with you that > > > >> nanoTime() is better here, but it is definitely not a reason for > > > mentioned > > > >> failure, since that test is launched in single JVM on a machine that > > > most > > > >> probably does not do any ntp syncs during the test to make Ignite's > > > >> timeout > > > >> machinery fail. > > > >> > > > >> Please file a ticket to switch Ignite's timeouts to nanoTime() at > some > > > >> point. > > > >> > > > >> --Yakov > > > >> > > > > > > > > > > > > > -- > Nikolay Izhikov > nizhikov....@gmail.com >
