Message ID | 20230817045433.495995-1-jtilahun@astranis.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | scripts: use the monotonic clock to compute time elapsed | expand |
On 17.08.23 at 06:54, jtilahun via lists.yoctoproject.org wrote: > From: Joseph Tilahun <jtilahun@astranis.com> > > The monotonic clock is preferable over the system clock when > computing the time elapsed. I agree. Otherwise, if you suspend your laptop, the time information counts the time spent sleeping. I submitted a patch to BitBake a while ago, but there was the fear that my change would break other parts (though it was working fine in my tests). However, I believe you should send your patch to openembedded-core@lists.openembedded.org instead, because that's a change for the openembedded-core repository. The "poky" repository is an aggregation of multiple original repositories. Thanks! Michael.
On Thu, 2023-08-17 at 08:35 +0200, Michael Opdenacker via lists.yoctoproject.org wrote: > On 17.08.23 at 06:54, jtilahun via lists.yoctoproject.org wrote: > > From: Joseph Tilahun <jtilahun@astranis.com> > > > > The monotonic clock is preferable over the system clock when > > computing the time elapsed. > > > I agree. Otherwise, if you suspend your laptop, the time information > counts the time spent sleeping. > I submitted a patch to BitBake a while ago, but there was the fear that > my change would break other parts (though it was working fine in my tests). The issue was that the python manual page clearly says: """ The reference point of the returned value is undefined, so that only the difference between the results of two calls is valid. """ and the timestamps being sent out by parts of bitbake may be used as absolute time values (e.g. displayed to the user), not just for comparison. Cheers, Richard
Thank you for the background Michael. I sent this patch to openembedded-core@lists.openembedded.org per your message. Richard, I understand the point you cite from the Python manual page. Regarding this patch specifically, only the difference between the results of two calls is used. So the concern you raise is not applicable to this patch specifically. Given that, I'd appreciate it if you could take a look. Thanks, Joseph On Thu, Aug 17, 2023 at 12:29 AM Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Thu, 2023-08-17 at 08:35 +0200, Michael Opdenacker via > lists.yoctoproject.org wrote: > > On 17.08.23 at 06:54, jtilahun via lists.yoctoproject.org wrote: > > > From: Joseph Tilahun <jtilahun@astranis.com> > > > > > > The monotonic clock is preferable over the system clock when > > > computing the time elapsed. > > > > > > I agree. Otherwise, if you suspend your laptop, the time information > > counts the time spent sleeping. > > I submitted a patch to BitBake a while ago, but there was the fear that > > my change would break other parts (though it was working fine in my > tests). > > The issue was that the python manual page clearly says: > > """ > The reference point of the returned value is undefined, so that only > the difference between the results of two calls is valid. > """ > > and the timestamps being sent out by parts of bitbake may be used as > absolute time values (e.g. displayed to the user), not just for > comparison. > > Cheers, > > Richard > > >
On Thu, 2023-08-17 at 13:28 -0700, Joseph Tilahun wrote: > Thank you for the background Michael. I sent this patch to > openembedded-core@lists.openembedded.org per your message. > > Richard, I understand the point you cite from the Python manual page. > Regarding this patch specifically, only the difference between the > results of two calls is used. So the concern you raise is not > applicable to this patch specifically. Given that, I'd appreciate it > if you could take a look. Alex's point stands though and relates to my concern. If we use time.time() everywhere we're consistent and less likely to confuse the two and get interesting results. Is there a real world problem this change is solving? Did you have a build machine where you changed the time part way through a build? I'm leaning towards being consistent as this time change would seem to be something very rare compared to the risks of people copy and pasting and confusing the usage. Cheers, Richard
The point about consistency is fair enough. time.time() is already used in many places, so it's understandable if the desire is to continue using time.time() throughout. I do think these usages of time in scripts are isolated enough to where it's not as much of a concern. The time deltas are computed purely within those functions and simply printed out, with nothing else done with that information. That being said, if you still insist that monotonic is not the way to go for the sake of consistency, then fine. Thanks, Joseph On Thu, Aug 17, 2023 at 1:32 PM Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Thu, 2023-08-17 at 13:28 -0700, Joseph Tilahun wrote: > > Thank you for the background Michael. I sent this patch to > > openembedded-core@lists.openembedded.org per your message. > > > > Richard, I understand the point you cite from the Python manual page. > > Regarding this patch specifically, only the difference between the > > results of two calls is used. So the concern you raise is not > > applicable to this patch specifically. Given that, I'd appreciate it > > if you could take a look. > > Alex's point stands though and relates to my concern. If we use > time.time() everywhere we're consistent and less likely to confuse the > two and get interesting results. > > Is there a real world problem this change is solving? Did you have a > build machine where you changed the time part way through a build? > > I'm leaning towards being consistent as this time change would seem to > be something very rare compared to the risks of people copy and pasting > and confusing the usage. > > Cheers, > > Richard >
diff --git a/scripts/oe-pkgdata-browser b/scripts/oe-pkgdata-browser index c152c82b25..727803ba93 100755 --- a/scripts/oe-pkgdata-browser +++ b/scripts/oe-pkgdata-browser @@ -29,10 +29,10 @@ FileColumns = enum.IntEnum("FileColumns", {"Filename": 0, "Size": 1}) import time def timeit(f): def timed(*args, **kw): - ts = time.time() + ts = time.monotonic() print ("func:%r calling" % f.__name__) result = f(*args, **kw) - te = time.time() + te = time.monotonic() print ('func:%r args:[%r, %r] took: %2.4f sec' % \ (f.__name__, args, kw, te-ts)) return result diff --git a/scripts/runqemu b/scripts/runqemu index 0e105a918b..d3c0b3fc38 100755 --- a/scripts/runqemu +++ b/scripts/runqemu @@ -1252,9 +1252,9 @@ to your build configuration. if self.snapshot and tmpfsdir: newrootfs = os.path.join(tmpfsdir, os.path.basename(self.rootfs)) + "." + str(os.getpid()) logger.info("Copying rootfs to %s" % newrootfs) - copy_start = time.time() + copy_start = time.monotonic() shutil.copyfile(self.rootfs, newrootfs) - logger.info("Copy done in %s seconds" % (time.time() - copy_start)) + logger.info("Copy done in %s seconds" % (time.monotonic() - copy_start)) self.rootfs = newrootfs # Don't need a second copy now! self.snapshot = False