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

Cannot represent instants before the program start #45

Open
Rodrigodd opened this issue Apr 22, 2022 · 7 comments · May be fixed by #46
Open

Cannot represent instants before the program start #45

Rodrigodd opened this issue Apr 22, 2022 · 7 comments · May be fixed by #46

Comments

@Rodrigodd
Copy link

In one of my projects, I have the necessity of representing an arbitrary instant in time, which include instants before the program start.

The current implementation relies on creating a Duration from performe.now(). Because performe.now() return the time since the creation of the page/web worker, and because Duration cannot represent negative values, the Wasm version of Instant can only represent a small range of past instants.

@Rodrigodd Rodrigodd linked a pull request Apr 24, 2022 that will close this issue
@daxpedda
Copy link

Is wasn't able to reproduce this and was also not able to find any information on it, could you elaborate how this can happen?

@Rodrigodd
Copy link
Author

@daxpedda This line panics with 'overflow when subtracting durations' (if your program has not being running for a 100s):

let _ = Instant::now() - Duration::from_secs(100);

I added a test to my PR to test this.

@daxpedda
Copy link

daxpedda commented Mar 27, 2023

Isn't this as intended? std::time::Instant should behave the same, see source.

So the behavior seems correct to me.

@Rodrigodd
Copy link
Author

The std implementation only panics if you subtract something like u64::MAX seconds from Instant::now(), not just any Duration greater than the program current execution time.

@daxpedda
Copy link

daxpedda commented Mar 27, 2023

Well yes, but that is not behavior that you should rely on. The starting point of the underlying clock is not defined by Rust.

The reason this currently works on Linux is because std uses clock_gettime:

All implementations support the system-wide realtime clock, which is identified by CLOCK_REALTIME. Its time represents seconds and nanoseconds since the Epoch.

So the starting time is since the UNIX epoch. So unless your computer boots with a time set to 1970 it will always be much bigger then 0.

On Windows for example, std uses QueryPerformanceCounter, which doesn't start from the UNIX epoch, 0 can be the start of when you boot. So the example you posted above might fail on Windows.


That said, looking at your PR, it seems to me that probably the better way to do this is to add Performance.timeOrigin to the calculation instead of shifting the whole number to an arbitrary point.

In any case, from a general-purpose library standpoint, an additional calculation is additional overhead for a non-conforming use-case, imho.

@Rodrigodd
Copy link
Author

Yes, you are right, on Windows I can get my example to panic with a value as little as 20_000 seconds, which matches with the time since boot behavior. So I should not really be relying on that.

I could argue that they should be able to handle negative values (because both Windows and POSIX appear to return signed values), but some platforms may not, and that is also beyond this issue.

So fell free to close this issue.

@daxpedda
Copy link

Apologies I gave the wrong impression here, I'm not a maintainer of this repo.

However, shameless plug, I was interested in this issue because I recently released web-time to tackle some of the issues in this repo.

Rodrigodd added a commit to Rodrigodd/gameroy that referenced this issue Apr 30, 2023
Previously, I used a Instant keeping the theorical time that the gameboy
was turned on to calculate the ammount of clocks in order to emulate the
gameboy in real time. Every time I fast-forward the emulate I recomputed
the start Instant to `Instant::now() - clock_to_duration(gb.clock_count)`,
getting a value of Instant in the past.

But it is not guarranteed that Instant is able to represent times in the
past and may panic on underflow (See sebcrozet/instant#45).

This is fixed by keeping `last_start_time: Instant` as the time that
the gameboy started running at the clock count `last_start_clock`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants