New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to nanosecond precision in Duration-based operators #1734
Comments
@william-tran I expect you're not using a default Unfortunately this assumption that the sub-millisecond precision isn't needed is pervasive in |
I've been spending the last half hour looking at how pervasive this is, and I see what you mean (I may have missed a few): reactor.core.publisher.Mono.delay(Duration, Scheduler)
reactor.core.publisher.Mono.block(Duration)
reactor.core.publisher.Mono.blockOptional(Duration)
reactor.core.publisher.Mono.delayElement(Duration, Scheduler)
reactor.core.publisher.Flux.interval(Duration, Scheduler)
reactor.core.publisher.Flux.interval(Duration, Duration, Scheduler)
reactor.core.publisher.Flux.blockFirst(Duration)
reactor.core.publisher.Flux.blockLast(Duration)
reactor.core.publisher.Flux.bufferTimeout(int, Duration, Scheduler, Supplier<C>) and FluxBufferTimeout
reactor.core.publisher.Flux.replay(int, Duration, Scheduler) and FluxReplay
reactor.core.publisher.Flux.timeout(Duration, Publisher<? extends T>, Scheduler)
reactor.core.publisher.Flux.windowTimeout(int, Duration, Scheduler) and FluxWindowTimeout
reactor.core.publisher.MonoCacheTime.CoordinatorSubscriber.signalCached(Signal<T>) But it looks like reactor.core.publisher.Flux.delaySequence(Duration) and FluxDelaySequence deals with nanos!
I was using |
I also looked at |
my main point is that So But maybe it would be possible to come up with a |
Even without real-time execution guarantees, the aggregate result of using nanos instead of millis for delays under 1 milli, is far better than the milli round-down resulting in no delay. The example |
I think I have an easy workaround for |
ok. I think we can switch to a I'll reword the issue. This would be a slight behavioral change though, and thus would target the 3.3 generation. We are about to release @smaldini wdyt about the switch from |
one way of doing it is by converting anything bigger than a minute to millis, like in if (delay.compareTo(Duration.ofMinutes(1)) < 0) {
this.delay = delay.toNanos();
this.timeUnit = TimeUnit.NANOSECONDS;
}
else {
this.delay = delay.toMillis();
this.timeUnit = TimeUnit.MILLISECONDS;
} Another approach would be to check for max nanos: static final Duration MAX_NANOS = Duration.ofNanos(Long.MAX_VALUE);
//then...
if (timeout.compareTo(MAX_NANOS)>=0) {
this.delay = delay.toMillis();
this.timeUnit = TimeUnit.MILLISECONDS;
}
else {
this.delay = delay.toNanos();
this.timeUnit = TimeUnit.NANOSECONDS;
} Any better idea? Approach 1 or 2? |
Approach 2 provides the most precision over the widest range of possible values, so I'd go with that. Approach 1 feels like an arbitrary point at which you decide to round off. 3.3 sounds like the right release, will there be an M3 that we can get this into? I'm happy to help with a PR, but not sure if I'll have the time to have it done for Monday. |
There should be a M3 within a month or so, yes. |
@william-tran we're still on the fence about making that change in 3.3 at all. Is there a real benefit right now that can be more fully fleshed out? I still think this makes sense in the near future, because JVMs might become more precise and we would then automatically benefit from that, but since nothing has been done yet and M3 is approaching, I'm just wondering if we can defer to further down the line (ie. 3.4). |
Wow does time ever fly. Because I have a workaround for nanosecond precision delay with |
@william-tran haha yeah, great to hear you have a working workaround 👍 we're not exactly depending on a contribution from you, but it would indeed be greatly appreciated 🎁 |
one thing I didn't think about in detail with making this move to NANOS is that it would introduce a discrepancy between what you can supposedly do eg in terms of And since java scheduling tools like I'm wondering if it would make sense to round up any strictly positive yet sub-millisecond duration to 1 millisecond? |
Ok, it sounds like a bit more trouble than it's worth for now. As for rounding, I would lean toward more explicitly documenting the fact that sub-millisecond decimals are truncated rather than rounding up for duration's in that range. That way, all duration's 0 to N milliseconds have consistent behavior and the user can decide how to proceed (not using |
Expected behavior
Mono.delay(Duration.ofMillis(1).minusNanos(1))
delays emission by 1 millisecond minus one nanosecondActual behavior
The Mono emits immediately without delay, because
Proposed fix
Use Case
I'm using Reactor to drive high concurrency simulations needing precision better than a millisecond.
The text was updated successfully, but these errors were encountered: