Possible performance regression in 3.5.0

[lbialy]

There's some discussion regarding the possible performance regression between 3.4.x and 3.5.0 found by Flavio Brasil. More information (including flame charts) can be found in his tweets here and replies in threads underneath it:

https://twitter.com/fbrasisil/status/1667798762788646912

Is it a regression or is it a part of the plan for I/O polling?

[djspiewak]

So basically, this is known, but we were planning on lazily-evaluating the optimization options when someone complains. :-) In 3.5.0, we call System.nanoTime() on each step of the WorkerThread loop. This gives us amazing timer granularity (far better than 3.4.x and prior), but it's also somewhat costly. Our benchmarks suggested that it wasn't much of a problem outside of microbenchmarks, so we left it alone to see if anyone would complain.

The easy optimization is to shift it into just state 0 of the worker. This would cause our timer granularity to get a bit worse, but still better than pre-3.5, and it would reduce the amortized cost of the syscall by a factor of 64. So… probably worth it? Not at all hard to do, we just didn't.

Call for opinions! Anyone have strong feelings about this one way or another?

[armanbilge]

Cross-linking to:

• Avoid per-iteration read barriers in worker loop #3544 (comment)
  where we also discuss reducing the timer granularity

• https://docs.libuv.org/en/v1.x/loop.html#c.uv_update_time
  which discusses libuv's timer caching strategy. we could do something similar here for the worker loop, but update it whenever there is an IO.monotonic call in userland

[lbialy]

Thanks for the explanation @djspiewak! I'll leave the issue open so that it's easy to find should anyone actually have a production issue with this.

[alexandru]

Speaking of System.nanoTime, not familiar with how that works, but it seems like a system call, and system calls can be unpredictable. It might be worth some optimization, if possible; even if microbenchmarks are not necessarily a reason for concern. Just an uninformed opinion/feeling at this point.

[djspiewak]

It is indeed a syscall. :-) It delegates to gettime on most platforms.

[SethTisue]

Should this be closed, since #3690 was merged?

[armanbilge]

GitHub doesn't auto-close issues if the PRs are not merged to the default branch...