[crac] RFR: Correct System.nanotime() value after restore [v8]

[Jan Kratochvil]

On Fri, 12 May 2023 08:46:14 GMT, Radim Vansa <duke at openjdk.org> wrote:

>> There are various places both inside JDK and in libraries that rely on monotonicity of `System.nanotime()`. When the process is restored on a different machine the value will likely differ as the implementation provides time since machine boot. This PR records wall clock time before checkpoint and after restore and tries to adjust the value provided by nanotime() to reasonably correct value.
>
> Radim Vansa has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 20 commits:
> 
>  - Merge branch 'crac' into nanotime
>  - Do not use negative monotonic offset
>  - Merge branch 'crac' into nanotime
>  - Fix whitespaces
>  - Use image under ghcr.io/crac
>  - Ensure monotonicity for the same boot
>  - Set nanotime only if bootid changes
>  - Reset nanotime offset before calculating it again
>  - Correct time since restore
>  - Merge remote-tracking branch 'origin/crac' into nanotime
>  - ... and 10 more: https://git.openjdk.org/crac/compare/4b0dc2dc...4c850312

src/hotspot/os/linux/os_linux.cpp line 6400:

> 6398: }
> 6399: 
> 6400: bool os::read_bootid(char *dest, size_t size) {

The `size` parameter is not useful when it must be always `UUID_LENGTH + 1`.

src/hotspot/os/linux/os_linux.cpp line 6421:

> 6419:     }
> 6420:     rd += r;
> 6421:   } while (rd < size - 1);

I would expect it would be good to check the file really has the expected length and the UUID ends with `'\n'`.

src/hotspot/os/linux/os_linux.cpp line 6422:

> 6420:     rd += r;
> 6421:   } while (rd < size - 1);
> 6422:   dest[size - 1] = '\0';

When the function supports only `UUID_LENGTH` files it does not make much sense to zero-terminate it. It is easier to rather do `memcmp` of `UUID_LENGTH' bytes.

src/hotspot/os/linux/os_linux.cpp line 6423:

> 6421:   } while (rd < size - 1);
> 6422:   dest[size - 1] = '\0';
> 6423:   ::close(fd);

Error check of `close()`?

src/hotspot/share/runtime/os.cpp line 2051:

> 2049:   // won't offset the time based on wall-clock time as this change in monotonic
> 2050:   // time is likely intentional.
> 2051:   if (!read_bootid(buf, sizeof(buf)) || strncmp(buf, checkpoint_bootid, UUID_LENGTH) != 0) {

Currently `read_bootid` always zero-terminates the string. Therefore it is easier to use just `strcmp`. When I proposed removing the zero-termination one would rather use `memcmp` instead of `strncmp`.

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

PR Review Comment: https://git.openjdk.org/crac/pull/53#discussion_r1193925337
PR Review Comment: https://git.openjdk.org/crac/pull/53#discussion_r1193927193
PR Review Comment: https://git.openjdk.org/crac/pull/53#discussion_r1193928930
PR Review Comment: https://git.openjdk.org/crac/pull/53#discussion_r1193931013
PR Review Comment: https://git.openjdk.org/crac/pull/53#discussion_r1193930500