Re: IgniteUtils#currentTimeMillis

2017-08-11 Thread Yakov Zhdanov
Well, then let's leave it as is for now. --Yakov

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Vladimir Ozerov
Yakov, For example: http://pzemtsov.github.io/2017/07/23/the-slow-currenttimemillis.html >>> We’ve learned that the slow execution of currentTimeMillis() was caused by two factors: >>> - JVM using gettimeofday() instead of clock_gettime() >>> - gettimeofday() being very slow if HPET time source i

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Evgeniy Stanilovskiy
I assume that Vladimir mention this mesurements: https://shipilev.net/blog/2014/nanotrusting-nanotime/ can we simple measure with JMH x86 and arm our realization vs system call? As Dmitry P mentioned System.currentTimeMillis() is JVM intrinsic. Moreover, there is a daemon thread that updates the

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Yakov Zhdanov
As Dmitry P mentioned System.currentTimeMillis() is JVM intrinsic. Moreover, there is a daemon thread that updates the internal value which will not be needed after the change. If we remove U.currentTimeMillis() code will become more clear and consistent. Why we think that we can implement this pa

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Alexey Kuznetsov
Nikolay, As far as I understand U.currentTimeMillis() should be used where time is not a major value (metrics for example). But in test with transaction (that you are mentioned) we should use System.currentTimeMillis(). In general we should think about U.currentTimeMillis() and avoid it usage in

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Vladimir Ozerov
In short, the reason is avoiding potential performance problems. On Wed, Aug 9, 2017 at 3:49 PM, Николай Ижиков wrote: > Dmitry, > > > So, if you change the call to System.currentTimeMillis(), the test > passes? > > Yes > > > I would propose to either increase TX_TIMEOUT or sleep multiplier to m

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Николай Ижиков
Dmitry, > So, if you change the call to System.currentTimeMillis(), the test passes? Yes > I would propose to either increase TX_TIMEOUT or sleep multiplier to make test more reliable. Yes, I fix test in that way. For me the goal of this discussion is to understand reasons to keep current meth

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Dmitriy Setrakyan
On Wed, Aug 9, 2017 at 5:32 AM, Николай Ижиков wrote: > Vladimir, > > As far as I can understand behaviour of U.currentTimeMillis() breaks > transaction timeout test: > So, if you change the call to System.currentTimeMillis(), the test passes?

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Vladimir Ozerov
I would propose to either increase TX_TIMEOUT or sleep multiplier to make test more reliable. On Wed, Aug 9, 2017 at 3:32 PM, Николай Ижиков wrote: > Vladimir, > > As far as I can understand behaviour of U.currentTimeMillis() breaks > transaction timeout test: > > https://issues.apache.org/jira/

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Николай Ижиков
Vladimir, As far as I can understand behaviour of U.currentTimeMillis() breaks transaction timeout test: https://issues.apache.org/jira/browse/IGNITE-5963 IgniteOptimisticTxSuspendResumeTest#testTxTimeoutOnSuspend : ``` final Transaction tx = ignite.transactions().txStart(OPTIMISTIC, isolation,

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Vladimir Ozerov
Guys, Are you really suggesting change the product for newcomer needs? This is not an argument for the change. Can someone explain me what is currently broken from product's perspective? Yes, we can get stale values for some time, we know that. Does it break something? On Wed, Aug 9, 2017 at 3:11

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Dmitry Pavlov
+1 to Nikolay, this is very often question from newcomers. It is not clear that current* method may return outdated value from some moment in past. Nikolay, how long outdated value can be returned by method? ср, 9 авг. 2017 г. в 15:00, Николай Ижиков : > Vladimir, > > > There is nothing wrong wi

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Dmitriy Setrakyan
On Wed, Aug 9, 2017 at 4:59 AM, Николай Ижиков 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 agre

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Николай Ижиков
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. 2017-08-09 14:55 GMT+03:00 Vladimir Ozerov : > You cannot check it with bench

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Vladimir Ozerov
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.

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Николай Ижиков
Hello, Dima. > Nickolay, could you please raise standalone ticket for U.currentTimeMillis () ? Yes, I can. > https://issues.apache.org/jira/browse/IGNITE-5963 I will try to make quick fix for issue without change Ignite core code. So fix will not depend to U.currentTimeMillis() implementation.

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Dmitry Pavlov
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 : > It seems System.currentTimeMillis () is now in intrinsic list. This means > on modern JVMs performanc

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Dmitry Pavlov
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 h

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Vladimir Ozerov
Java could do some heavy stuff when doing currentTimeMillis, depending on the platform or vendor. I remember I saw some articles about performance issues caused by currentTimeMillis (something about high contention on certain OS configuration). So I do not see a reason why we should remove it. It i

Re: IgniteUtils#currentTimeMillis

2017-08-09 Thread Yakov Zhdanov
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?

Re: IgniteUtils#currentTimeMillis

2017-08-07 Thread Николай Ижиков
Addition to my previous message: 1. IgniteUtils#currentTimeMillis used in current master to check transaction timeout: https://github.com/apache/ignite/blob/master/modules/ core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/ IgniteTxAdapter.java#L664 2. According to jdk