On Thu, Feb 27, 2020 at 11:26 PM Christopher Schultz < ch...@christopherschultz.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > Rémy, > > On 2/27/20 17:20, Christopher Schultz wrote: > > Rémy, > > > > On 2/27/20 10:49, r...@apache.org wrote: > >> This is an automated email from the ASF dual-hosted git > >> repository. > > > >> remm pushed a commit to branch master in repository > >> https://gitbox.apache.org/repos/asf/tomcat.git > > > > > >> The following commit(s) were added to refs/heads/master by this > >> push: new 22ad695 Update request start time using nanoTime > >> 22ad695 is described below > > > >> commit 22ad69571c019f4d84ccc522298dddb4f8fa8d70 Author: remm > >> <r...@apache.org> AuthorDate: Thu Feb 27 16:49:04 2020 +0100 > > > >> Update request start time using nanoTime > > > >> get/setStartTime are still there, not sure about existing use. > >> Another patch round could deprecate them. Also change the access > >> log to be the start of the request (a small part of 63286). --- > > > >> [snip] > > > >> diff --git a/java/org/apache/coyote/AbstractProcessor.java > >> b/java/org/apache/coyote/AbstractProcessor.java index > >> 254950e..5af3710 100644 --- > >> a/java/org/apache/coyote/AbstractProcessor.java +++ > >> b/java/org/apache/coyote/AbstractProcessor.java @@ -978,6 +978,7 > >> @@ public abstract class AbstractProcessor extends > >> AbstractProcessorLight implement > >> setSocketWrapper(socketWrapper); // Setup the minimal request > >> information request.setStartTime(System.currentTimeMillis()); + > >> request.setStartTimeNanos(System.nanoTime()); // Setup the > >> minimal response information response.setStatus(400); > >> response.setError(); > > > > Since we are talking about nanoseconds, here, we ARE, by > > definition, splitting hairs :) > > > > System.currentTimeMillis() and System.nanoTime() may disagree when > > you capture them like this, because [a super-small amount of] time > > elapses between the two calls. > > > > Should we instead do: > > > > long nanos = System.nanoTime(); request.setStartTimeNanos(nanos); > > request.setStartTime(nanos / 1000000l); > > > > Or maybe: > > > > long nanos = System.nanoTime(); request.setStartTimeNanos(nanos); > > request.setStartTime(TimeUnit.NANOSECONDS.toMillis(nanos)); > > > > ? > > Also, System.nanoTime apparently takes a lot longer than > System.currentTimeMillis(), but it always moves forward in time, while > System.currentTimeMillis can fall back for leap seconds, etc. and can > behave in surprising ways :) > getStartTime isn't used anymore [in Tomcat itself] so it's not a big problem. I'll try to think more about removing it, and maybe Mark will want to redo this refactoring anyway. Rémy > > - -chris > -----BEGIN PGP SIGNATURE----- > Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/ > > iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl5YQhgACgkQHPApP6U8 > pFgh4g/9GPUk5r+Lu8F2+JdhBs35v1oZwJE7D341sOcgycbVPLzoWBh2/EJvNSUS > l+PmfkVSJNNUqMgjQU0UAtQBNEkvanTA+teGP1UyJZS1tL6dkrJAQslXbO0ReUYX > sRzo9xHSrrBhc5CTYpfryHzEuf798zNBuQvyze84eyCT3ZQeaYm34JbQ6/QPh9XV > GBFMWyQXK8tm7JdGrPpd9QRmTxvTyVdq327bUv4fcYnjv0OUS67illZQWCKMv1JK > DwJpVDMzL/RtoaxGJWREuxC0L6541czgD6tEOV2dxUYSUR4OnTQAhPOr/4NvARH7 > JV3j3qzV3jiqDSwHePplAKk/i03nOTRq6wZW/D76Yg4Ut9C1p1wWWF6Dw2kTT724 > DNpcSKYHA2t4Mg09z/C7dr4gtXYQ4SbxFtvEseiHycKq5ViWpGUYs83DQjMnMwBW > /Fw1JnZulMWxz5UkmHM+fSAsAF/njLAFBVY7cf8xhBoPok6coyOBCcsS46ymizde > notzIXpiuWOAaqP8g1Bjzm3OYjjH2gOxKLXhfghpy7guG5NGfuL9E+KDF90KzL70 > dZsTepF0d/slGkDP/g5f9PSYwZhwsz7jAkaQbSAm68EjF1MsAQbnv+MxSBthOOP1 > y9J6g9SqCakWe8JyUrHTmtO39EMugBvW75OubbLP3c/JyIdDi94= > =aLrh > -----END PGP SIGNATURE----- > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
