code review 161110044: runtime: warn that cputicks() might not be monotonic.

runtime: warn that cputicks() might not be monotonic.

Get rid of gocputicks(), it is no longer used.

[khr, 2014-10-21 21:06:50]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://khr%40golang.org@code.google.com/p/go/

[bradfitz, 2014-10-21 21:11:07]

LGTM
 On Oct 21, 2014 11:06 PM, <khr@golang.org> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://khr%40golang.org@code.google.com/p/go/
>
>
> Description:
> runtime: warn that cputicks() might not be monotonic.
>
> Get rid of gocputicks(), it is no longer used.
>
> Please review this at https://codereview.appspot.com/161110044/
>
> Affected files (+4, -23 lines):
>   M src/runtime/asm_386.s
>   M src/runtime/asm_amd64.s
>   M src/runtime/asm_amd64p32.s
>   M src/runtime/asm_arm.s
>   M src/runtime/stubs.go
>
>
> Index: src/runtime/asm_386.s
> ===================================================================
> --- a/src/runtime/asm_386.s
> +++ b/src/runtime/asm_386.s
> @@ -871,12 +871,6 @@
>         MOVL    DX, ret_hi+4(FP)
>         RET
>
> -TEXT runtime·gocputicks(SB),NOSPLIT,$0-8
> -       RDTSC
> -       MOVL    AX, ret_lo+0(FP)
> -       MOVL    DX, ret_hi+4(FP)
> -       RET
> -
>  TEXT runtime·ldt0setup(SB),NOSPLIT,$16-0
>         // set up ldt 7 to point at tls0
>         // ldt 1 would be fine on Linux, but on OS X, 7 is as low as we
> can go.
> Index: src/runtime/asm_amd64.s
> ===================================================================
> --- a/src/runtime/asm_amd64.s
> +++ b/src/runtime/asm_amd64.s
> @@ -855,13 +855,6 @@
>         MOVQ    AX, ret+0(FP)
>         RET
>
> -TEXT runtime·gocputicks(SB),NOSPLIT,$0-8
> -       RDTSC
> -       SHLQ    $32, DX
> -       ADDQ    DX, AX
> -       MOVQ    AX, ret+0(FP)
> -       RET
> -
>  // hash function using AES hardware instructions
>  TEXT runtime·aeshash(SB),NOSPLIT,$0-32
>         MOVQ    p+0(FP), AX     // ptr to data
> Index: src/runtime/asm_amd64p32.s
> ===================================================================
> --- a/src/runtime/asm_amd64p32.s
> +++ b/src/runtime/asm_amd64p32.s
> @@ -657,13 +657,6 @@
>         MOVQ    AX, ret+0(FP)
>         RET
>
> -TEXT runtime·gocputicks(SB),NOSPLIT,$0-8
> -       RDTSC
> -       SHLQ    $32, DX
> -       ADDQ    DX, AX
> -       MOVQ    AX, ret+0(FP)
> -       RET
> -
>  // hash function using AES hardware instructions
>  // For now, our one amd64p32 system (NaCl) does not
>  // support using AES instructions, so have not bothered to
> Index: src/runtime/asm_arm.s
> ===================================================================
> --- a/src/runtime/asm_arm.s
> +++ b/src/runtime/asm_arm.s
> @@ -1275,9 +1275,6 @@
>         MOVW    R0, ret+0(FP)
>         RET
>
> -TEXT runtime·gocputicks(SB),NOSPLIT,$0
> -       B runtime·cputicks(SB)
> -
>  TEXT runtime·return0(SB),NOSPLIT,$0
>         MOVW    $0, R0
>         RET
> Index: src/runtime/stubs.go
> ===================================================================
> --- a/src/runtime/stubs.go
> +++ b/src/runtime/stubs.go
> @@ -180,7 +180,11 @@
>  func breakpoint()
>  func nanotime() int64
>  func usleep(usec uint32)
> +
> +// careful: cputicks is not guaranteed to be monotonic!  In particular,
> we have
> +// noticed drift between cpus on certain os/arch combinations.
>  func cputicks() int64
> +
>  func mmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off
> uint32) unsafe.Pointer
>  func munmap(addr unsafe.Pointer, n uintptr)
>  func madvise(addr unsafe.Pointer, n uintptr, flags int32)
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[dave_cheney.net, 2014-10-21 21:28:05]

LGTM
On 22 Oct 2014 08:06, <khr@golang.org> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://khr%40golang.org@code.google.com/p/go/
>
>
> Description:
> runtime: warn that cputicks() might not be monotonic.
>
> Get rid of gocputicks(), it is no longer used.
>
> Please review this at https://codereview.appspot.com/161110044/
>
> Affected files (+4, -23 lines):
>   M src/runtime/asm_386.s
>   M src/runtime/asm_amd64.s
>   M src/runtime/asm_amd64p32.s
>   M src/runtime/asm_arm.s
>   M src/runtime/stubs.go
>
>
> Index: src/runtime/asm_386.s
> ===================================================================
> --- a/src/runtime/asm_386.s
> +++ b/src/runtime/asm_386.s
> @@ -871,12 +871,6 @@
>         MOVL    DX, ret_hi+4(FP)
>         RET
>
> -TEXT runtime·gocputicks(SB),NOSPLIT,$0-8
> -       RDTSC
> -       MOVL    AX, ret_lo+0(FP)
> -       MOVL    DX, ret_hi+4(FP)
> -       RET
> -
>  TEXT runtime·ldt0setup(SB),NOSPLIT,$16-0
>         // set up ldt 7 to point at tls0
>         // ldt 1 would be fine on Linux, but on OS X, 7 is as low as we
> can go.
> Index: src/runtime/asm_amd64.s
> ===================================================================
> --- a/src/runtime/asm_amd64.s
> +++ b/src/runtime/asm_amd64.s
> @@ -855,13 +855,6 @@
>         MOVQ    AX, ret+0(FP)
>         RET
>
> -TEXT runtime·gocputicks(SB),NOSPLIT,$0-8
> -       RDTSC
> -       SHLQ    $32, DX
> -       ADDQ    DX, AX
> -       MOVQ    AX, ret+0(FP)
> -       RET
> -
>  // hash function using AES hardware instructions
>  TEXT runtime·aeshash(SB),NOSPLIT,$0-32
>         MOVQ    p+0(FP), AX     // ptr to data
> Index: src/runtime/asm_amd64p32.s
> ===================================================================
> --- a/src/runtime/asm_amd64p32.s
> +++ b/src/runtime/asm_amd64p32.s
> @@ -657,13 +657,6 @@
>         MOVQ    AX, ret+0(FP)
>         RET
>
> -TEXT runtime·gocputicks(SB),NOSPLIT,$0-8
> -       RDTSC
> -       SHLQ    $32, DX
> -       ADDQ    DX, AX
> -       MOVQ    AX, ret+0(FP)
> -       RET
> -
>  // hash function using AES hardware instructions
>  // For now, our one amd64p32 system (NaCl) does not
>  // support using AES instructions, so have not bothered to
> Index: src/runtime/asm_arm.s
> ===================================================================
> --- a/src/runtime/asm_arm.s
> +++ b/src/runtime/asm_arm.s
> @@ -1275,9 +1275,6 @@
>         MOVW    R0, ret+0(FP)
>         RET
>
> -TEXT runtime·gocputicks(SB),NOSPLIT,$0
> -       B runtime·cputicks(SB)
> -
>  TEXT runtime·return0(SB),NOSPLIT,$0
>         MOVW    $0, R0
>         RET
> Index: src/runtime/stubs.go
> ===================================================================
> --- a/src/runtime/stubs.go
> +++ b/src/runtime/stubs.go
> @@ -180,7 +180,11 @@
>  func breakpoint()
>  func nanotime() int64
>  func usleep(usec uint32)
> +
> +// careful: cputicks is not guaranteed to be monotonic!  In particular,
> we have
> +// noticed drift between cpus on certain os/arch combinations.
>  func cputicks() int64
> +
>  func mmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off
> uint32) unsafe.Pointer
>  func munmap(addr unsafe.Pointer, n uintptr)
>  func madvise(addr unsafe.Pointer, n uintptr, flags int32)
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[minux, 2014-10-21 21:29:02]

also consider linking to the issue? it has lots of useful information.

[khr, 2014-10-21 21:46:11]

*** Submitted as https://code.google.com/p/go/source/detail?r=663d8fa28048 ***

runtime: warn that cputicks() might not be monotonic.

Get rid of gocputicks(), it is no longer used.

LGTM=bradfitz, dave
R=golang-codereviews, bradfitz, dave, minux
CC=golang-codereviews
https://codereview.appspot.com/161110044

[gobot, 2014-10-21 21:48:12]

This CL appears to have broken the plan9-amd64-aram builder.
See http://build.golang.org/log/3be1d918ed8995e0f9768e55d77285bebf94639c