On Mon, 26 Feb 2024 17:40:59 GMT, Alan Bateman <al...@openjdk.org> wrote:

>> Is n == 0 intended to detect a spinning condition where the selector goes 
>> back into select when the event has not been handled?
>> 
>> In that case should we still emit an event if a timeout is present and the 
>> duration is greater than the timeout? For instance, in the HttpClient, we 
>> have a selector thread that will wake up at regular interval - we set up a 
>> timeout which is the min between the next expected timer event and 1500ms. 
>> So that could potentially fire an event every 1500ms if for instance the 
>> connection threshold is less than 1500ms and the connection is idle.
>> 
>> Maybe that's OK - and maybe in that case the onus is on the user to set a 
>> threshold greater than 1500ms?
>> 
>> Or should it be ((n == 0 && durationToMillis(duration) < 
>> timeoutToMillis(timeout)) || ...)) (duration and timeout probably are not in 
>> the same unit of time) - also if shouldCommit return false will the event 
>> actually be emitted down the road? Because if not then the ( n== 0 || ...) 
>> won't work.
>
>> Maybe that's OK - and maybe in that case the onus is on the user to set a 
>> threshold greater than 1500ms?
> 
> The threshold is 20ms so these timed-select ops in the HTTP client will 
> record an event when they timeout.

We should probably find a way to not emit the event if n == 0 and the operation 
was interrupted by `Selector.wakeUp`. Since we have another issue logged to 
emit a spin event, I wonder if we should only commit the event here if `n != 
0`? The case where n == 0 would be handled by the spin event (added later) 
@AlanBateman what do you think?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16710#discussion_r1560693482

Reply via email to