On Mon, 9 Sep 2024 13:58:51 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:
>> Aleksei Efimov has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - guard against possible integer value overflows >> - make startTime a local variable > > src/jdk.naming.dns/share/classes/com/sun/jndi/dns/DnsClient.java line 477: > >> 475: long elapsedMillis = >> TimeUnit.NANOSECONDS.toMillis(end - start); >> 476: // Setting the Math.clamp min to 1 ensures that the >> timeout is decreased >> 477: timeoutLeft = timeoutLeft - >> Math.clamp(elapsedMillis, 1, Integer.MAX_VALUE); > > I think I'd prefer to calculate the remaining timeout based on the total > elapsed time in this loop, as opposed to the time spent in blockingReceive. Sounds like a right thing to do: measuring time in the loop should give us better estimation on time DNS client spends waiting on the response after submiting a query (that's how environment property value is defined in [javadoc here](https://docs.oracle.com/en/java/javase/22/docs/api/jdk.naming.dns/module-summary.html)). I've tried to move `start` and `end` like: do { + long start = System.nanoTime(); <....> - long start = System.nanoTime(); gotData = blockingReceive(udpChannel, ipkt, timeoutLeft); - long end = System.nanoTime(); <...> + long end = System.nanoTime(); long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(end - start); As a result the tests showed that the timeout happened with a bit better precision (10th of milliseconds). I will run more tests and incorporate the suggestions. Thank you. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20892#discussion_r1750402311
