Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(139)

Issue 6905057: code review 6905057: os: Improve the accuracy of os.Chtimes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 8 months ago by Nick Craig-Wood
Modified:
9 years, 7 months ago
Reviewers:
CC:
golang-dev, brainman, rsc, bradfitz
Visibility:
Public.

Description

os: Improve the accuracy of os.Chtimes I've been writing some code which involves syncing files (like rsync) and it became apparent that under Linux I could read modification times (os.Lstat) with nanosecond precision but only write them with microsecond precision. This difference in precision is rather annoying when trying to discover whether files need syncing or not! I've patched syscall and os to increases the accuracy of of os.Chtimes for Linux and Windows. This involved exposing the utimensat system call under Linux and a bit of extra code under Windows. I decided not to expose the "at" bit of the system call as it is impossible to replicate under Windows, so the patch adds syscall.Utimens() to all architectures along with a ImplementsUtimens flag. If the utimensat syscall isn't available (utimensat was added to Linux in 2.6.22, Released, 8 July 2007) then it silently falls back to the microsecond accuracy version it uses now. The improved accuracy for Windows should be good for all versions of Windows. Unfortunately Darwin doesn't seem to have a utimensat system call that I could find so I couldn't implement it there. The BSDs do, but since they share their syscall implementation with Darwin I couldn't figure out how to define a syscall for *BSD and not Darwin. I've left this as a TODO in the code. In the process I implemented the missing methods for Timespec under Windows which I needed which just happened to round out the Timespec API for all platforms! ------------------------------------------------------------ Test code: http://play.golang.org/p/1xnGuYOi4b Linux Before (1000 ns precision) $ ./utimetest.linux.before z Setting mtime 1344937903123456789: 2012-08-14 10:51:43.123456789 +0100 BST Reading mtime 1344937903123457000: 2012-08-14 10:51:43.123457 +0100 BST Linux After (1 ns precision) $ ./utimetest.linux.after z Setting mtime 1344937903123456789: 2012-08-14 10:51:43.123456789 +0100 BST Reading mtime 1344937903123456789: 2012-08-14 10:51:43.123456789 +0100 BST Windows Before (1000 ns precision) X:\>utimetest.windows.before.exe c:\Test.txt Setting mtime 1344937903123456789: 2012-08-14 10:51:43.123456789 +0100 GMTDT Reading mtime 1344937903123456000: 2012-08-14 10:51:43.123456 +0100 GMTDT Windows After (100 ns precision) X:\>utimetest.windows.after.exe c:\Test.txt Setting mtime 1344937903123456789: 2012-08-14 10:51:43.123456789 +0100 GMTDT Reading mtime 1344937903123456700: 2012-08-14 10:51:43.1234567 +0100 GMTDT

Patch Set 1 #

Patch Set 2 : diff -r 6b602ab487d6 https://code.google.com/p/go #

Patch Set 3 : diff -r 6b602ab487d6 https://code.google.com/p/go #

Total comments: 1

Patch Set 4 : diff -r c031aa767edf https://code.google.com/p/go #

Total comments: 1

Patch Set 5 : diff -r afac768ad2fe https://code.google.com/p/go #

Total comments: 2

Patch Set 6 : diff -r ac06fe42df6d https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+127 lines, -6 lines) Patch
M src/pkg/os/file_posix.go View 1 2 3 1 chunk +4 lines, -6 lines 0 comments Download
M src/pkg/syscall/syscall_bsd.go View 1 2 3 4 5 1 chunk +15 lines, -0 lines 0 comments Download
M src/pkg/syscall/syscall_linux.go View 1 2 3 1 chunk +19 lines, -0 lines 0 comments Download
M src/pkg/syscall/syscall_windows.go View 1 2 3 2 chunks +28 lines, -0 lines 0 comments Download
M src/pkg/syscall/types_linux.go View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M src/pkg/syscall/zsyscall_linux_386.go View 1 1 chunk +15 lines, -0 lines 0 comments Download
M src/pkg/syscall/zsyscall_linux_amd64.go View 1 1 chunk +15 lines, -0 lines 0 comments Download
M src/pkg/syscall/zsyscall_linux_arm.go View 1 1 chunk +15 lines, -0 lines 0 comments Download
M src/pkg/syscall/ztypes_linux_386.go View 1 1 chunk +4 lines, -0 lines 0 comments Download
M src/pkg/syscall/ztypes_linux_amd64.go View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M src/pkg/syscall/ztypes_linux_arm.go View 1 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 23
Nick Craig-Wood
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
9 years, 8 months ago (2012-12-08 22:47:42 UTC) #1
brainman
It fails to build on windows with: # GOOS=windows go install -v syscall syscall # ...
9 years, 8 months ago (2012-12-10 06:25:25 UTC) #2
rsc
I haven't looked at the code but increasing the resolution of Chtimes is fine.
9 years, 8 months ago (2012-12-10 06:49:56 UTC) #3
bradfitz
https://codereview.appspot.com/6905057/diff/3012/src/pkg/os/file_posix.go File src/pkg/os/file_posix.go (right): https://codereview.appspot.com/6905057/diff/3012/src/pkg/os/file_posix.go#newcode156 src/pkg/os/file_posix.go:156: if syscall.ImplementsUtimens { I'm not sure I'm a fan ...
9 years, 8 months ago (2012-12-10 17:44:31 UTC) #4
rsc
It would be nice if we could avoid func init in package os if possible. ...
9 years, 8 months ago (2012-12-10 18:07:54 UTC) #5
Nick Craig-Wood
On 2012/12/10 17:44:31, bradfitz wrote: > https://codereview.appspot.com/6905057/diff/3012/src/pkg/os/file_posix.go > File src/pkg/os/file_posix.go (right): > > https://codereview.appspot.com/6905057/diff/3012/src/pkg/os/file_posix.go#newcode156 > ...
9 years, 8 months ago (2012-12-10 18:10:50 UTC) #6
Nick Craig-Wood
On 2012/12/10 06:25:25, brainman wrote: > It fails to build on windows with: [snip] > ...
9 years, 8 months ago (2012-12-10 18:12:50 UTC) #7
bradfitz
On Mon, Dec 10, 2012 at 1:07 PM, Russ Cox <rsc@golang.org> wrote: > It would ...
9 years, 8 months ago (2012-12-10 18:26:12 UTC) #8
rsc
Oh, I missed the syscall.NsecToTimeval. It seems fine to write a syscall.UtimesNano that just wraps ...
9 years, 8 months ago (2012-12-10 18:38:15 UTC) #9
Nick Craig-Wood
On 2012/12/10 18:38:15, rsc wrote: > Oh, I missed the syscall.NsecToTimeval. > It seems fine ...
9 years, 8 months ago (2012-12-10 20:56:27 UTC) #10
Nick Craig-Wood
Hello golang-dev@googlegroups.com, alex.brainman@gmail.com, rsc@golang.org, bradfitz@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 8 months ago (2012-12-10 23:28:54 UTC) #11
Nick Craig-Wood
On 2012/12/10 23:28:54, Nick Craig-Wood wrote: > Hello mailto:golang-dev@googlegroups.com, mailto:alex.brainman@gmail.com, mailto:rsc@golang.org, > mailto:bradfitz@golang.org (cc: mailto:golang-dev@googlegroups.com), ...
9 years, 8 months ago (2012-12-10 23:32:34 UTC) #12
brainman
On 2012/12/10 18:38:15, rsc wrote: > ... > It seems fine to write a syscall.UtimesNano ...
9 years, 8 months ago (2012-12-10 23:41:23 UTC) #13
Nick Craig-Wood
On 2012/12/10 23:41:23, brainman wrote: > On 2012/12/10 18:38:15, rsc wrote: > > ... > ...
9 years, 8 months ago (2012-12-11 14:58:36 UTC) #14
rsc
I think this CL is fine as is. Please don't put any more syscall-specific stuff ...
9 years, 8 months ago (2012-12-11 15:22:27 UTC) #15
Nick Craig-Wood
Hello golang-dev@googlegroups.com, alex.brainman@gmail.com, rsc@golang.org, bradfitz@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 8 months ago (2012-12-12 13:00:14 UTC) #16
Nick Craig-Wood
On 2012/12/11 15:22:27, rsc wrote: > I think this CL is fine as is. Please ...
9 years, 8 months ago (2012-12-12 13:01:57 UTC) #17
bradfitz
LGTM but should wait on Alex for Windows. https://codereview.appspot.com/6905057/diff/21001/src/pkg/syscall/syscall_bsd.go File src/pkg/syscall/syscall_bsd.go (right): https://codereview.appspot.com/6905057/diff/21001/src/pkg/syscall/syscall_bsd.go#newcode612 src/pkg/syscall/syscall_bsd.go:612: // ...
9 years, 8 months ago (2012-12-12 19:52:38 UTC) #18
brainman
LGTM Leaving for Brad to submit once you have made his changes. Alex
9 years, 8 months ago (2012-12-13 00:51:45 UTC) #19
Nick Craig-Wood
Hello golang-dev@googlegroups.com, alex.brainman@gmail.com, rsc@golang.org, bradfitz@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 8 months ago (2012-12-13 17:00:13 UTC) #20
Nick Craig-Wood
On 2012/12/12 19:52:38, bradfitz wrote: > LGTM > > but should wait on Alex for ...
9 years, 8 months ago (2012-12-13 17:01:41 UTC) #21
bradfitz
Nick, I don't see a CLA on file for you. See http://golang.org/doc/contribute.html#copyright Once a CLA ...
9 years, 8 months ago (2012-12-13 20:23:36 UTC) #22
bradfitz
9 years, 8 months ago (2012-12-13 21:02:43 UTC) #23
*** Submitted as https://code.google.com/p/go/source/detail?r=b7482cb67996 ***

os: Improve the accuracy of os.Chtimes

I've been writing some code which involves syncing files (like
rsync) and it became apparent that under Linux I could read
modification times (os.Lstat) with nanosecond precision but
only write them with microsecond precision.  This difference
in precision is rather annoying when trying to discover
whether files need syncing or not!

I've patched syscall and os to increases the accuracy of of
os.Chtimes for Linux and Windows.  This involved exposing the
utimensat system call under Linux and a bit of extra code
under Windows.  I decided not to expose the "at" bit of the
system call as it is impossible to replicate under Windows, so
the patch adds syscall.Utimens() to all architectures along
with a ImplementsUtimens flag.

If the utimensat syscall isn't available (utimensat was added
to Linux in 2.6.22, Released, 8 July 2007) then it silently
falls back to the microsecond accuracy version it uses now.
The improved accuracy for Windows should be good for all
versions of Windows.

Unfortunately Darwin doesn't seem to have a utimensat system
call that I could find so I couldn't implement it there.  The
BSDs do, but since they share their syscall implementation
with Darwin I couldn't figure out how to define a syscall for
*BSD and not Darwin.  I've left this as a TODO in the code.

In the process I implemented the missing methods for Timespec
under Windows which I needed which just happened to round out
the Timespec API for all platforms!

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

Test code: http://play.golang.org/p/1xnGuYOi4b

Linux Before (1000 ns precision)

$ ./utimetest.linux.before z
Setting mtime 1344937903123456789: 2012-08-14 10:51:43.123456789 +0100 BST
Reading mtime 1344937903123457000: 2012-08-14 10:51:43.123457 +0100 BST

Linux After (1 ns precision)

$ ./utimetest.linux.after z
Setting mtime 1344937903123456789: 2012-08-14 10:51:43.123456789 +0100 BST
Reading mtime 1344937903123456789: 2012-08-14 10:51:43.123456789 +0100 BST

Windows Before (1000 ns precision)

X:\>utimetest.windows.before.exe c:\Test.txt
Setting mtime 1344937903123456789: 2012-08-14 10:51:43.123456789 +0100 GMTDT
Reading mtime 1344937903123456000: 2012-08-14 10:51:43.123456 +0100 GMTDT

Windows After (100 ns precision)

X:\>utimetest.windows.after.exe c:\Test.txt
Setting mtime 1344937903123456789: 2012-08-14 10:51:43.123456789 +0100 GMTDT
Reading mtime 1344937903123456700: 2012-08-14 10:51:43.1234567 +0100 GMTDT

R=golang-dev, alex.brainman, rsc, bradfitz
CC=golang-dev
https://codereview.appspot.com/6905057

Committer: Brad Fitzpatrick <bradfitz@golang.org>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b