Skip to content
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

fix issue 21148, possible failure of CI when testing nano precision of file time stamps #7589

Merged
merged 4 commits into from Aug 13, 2020
Merged

Conversation

ghost
Copy link

@ghost ghost commented Aug 12, 2020

from man time:

       The software clock, HZ, and jiffies
       The accuracy of various system calls that set timeouts, (e.g.,
       select(2), sigtimedwait(2)) and measure CPU time (e.g., getrusage(2))
       is limited by the resolution of the software clock, a clock
       maintained by the kernel which measures time in jiffies.  The size of
       a jiffy is determined by the value of the kernel constant HZ.

       The value of HZ varies across kernel versions and hardware platforms.
       On i386 the situation is as follows: on kernels up to and including
       2.4.x, HZ was 100, giving a jiffy value of 0.01 seconds; starting
       with 2.6.0, HZ was raised to 1000, giving a jiffy of 0.001 seconds.
       Since kernel 2.6.13, the HZ value is a kernel configuration parameter
       and can be **100**, 250 (the default) or 1000, yielding a jiffies value
       of, respectively, 0.01, 0.004, or 0.001 seconds.  Since kernel
       2.6.20, a further frequency is available: 300, a number that divides
       evenly for the common video frame rates (PAL, 25 HZ; NTSC, 30 HZ).

       The times(2) system call is a special case.  It reports times with a
       granularity defined by the kernel constant USER_HZ.  User-space
       applications can determine the value of this constant using
       sysconf(_SC_CLK_TCK).

TL;DR

If the kernel setting for the software clock frequency (HZ) is set to 100, we got 10 msec resolution. CIs being on VMs we cant control it's better to assume that the setting can be set to this worst value, so test with a value greater than 10 msecs.

…f file time stamps

If the kernel setting for the software clock frequency (HZ) is set to 100, we got 10 msec resolution. CIs being on VMs we cant control it's better to assume that the setting can be set to this worst value, so test with a value greater than 10 msecs.
@ghost ghost requested a review from CyberShadow as a code owner August 12, 2020 20:38
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @NilsLankila!

Bugzilla references

Auto-close Bugzilla Severity Description
21148 normal Semaphoreci: core.exception.AssertError@std/file.d(1929): unittest failure

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#7589"

@ghost
Copy link
Author

ghost commented Aug 12, 2020

Another valid approach is to drop this test. What is tested here ? a kernel feature ? nano_sleep() ? the filesystem ? This tests nothing of phobos.

@CyberShadow
Copy link
Member

What is tested here ?

This is easily answerable using git-blame.

I believe the test is for the part of the timeLastModified implementation which queries non-standard (system-specific) fields of struct stat.

@WalterBright
Copy link
Member

I approve with the proviso that your explanation above be added as a comment to that test. Otherwise, that test will remain a mystery, not many people look at git blame.

And thanks for the quick action on this bug, @NilsLankila ! We're slowly bending the test suite into a state where a perfect PR is getting more than a 50% likelihood of passing all the tests.

Copy link
Member

@WalterBright WalterBright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the comment.

@ghost
Copy link
Author

ghost commented Aug 13, 2020

done, to be clear it is not sure that this will solve the failures, it's just that, according to the doc, a too tight sleep time can be a cause.

@ghost ghost requested a review from WalterBright August 13, 2020 05:19
Nils Lankila added 2 commits August 13, 2020 07:30
@ghost
Copy link
Author

ghost commented Aug 13, 2020

BTW if someone starts the merge train, dont forget to auto-merge-squash as I've made this PR online.

@dlang-bot dlang-bot merged commit e36b5aa into dlang:master Aug 13, 2020
@ghost ghost deleted the patch-1 branch August 13, 2020 08:12
nordlow pushed a commit to nordlow/phobos that referenced this pull request Sep 10, 2020
…f file time stamps (dlang#7589)

fix issue 21148, possible failure of CI when testing nano precision of file time stamps
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants