ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitriy Setrakyan <dsetrak...@apache.org>
Subject Re: IgniteUtils#currentTimeMillis
Date Wed, 09 Aug 2017 12:01:57 GMT
On Wed, Aug 9, 2017 at 4:59 AM, Николай Ижиков <nizhikov.dev@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 <vozerov@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.spb@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.spb@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 <yzhdanov@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.dev@gmail.com
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message