viml/profile: revert proftime_T to unsigned type #10521
Conversation
- reltimestr(): Produce negative value by comparing the unsigned proftime_T value to INT64_MAX. neovim#10452 (comment) 1. The interfaces of nearly all platforms return uint64_t. INT64_MAX is only half of that. 2. Low-level interfaces like this typically define that there is no fixed starting point. The only guarantees are that it's (a) monotonically increasing at a rate that (b) matches real time. ref 06af88c fix neovim#10452
justinmk
force-pushed
the
profile
branch
3 times, most recently
from
a5a112a
to
803749a
Compare
| /// | ||
| /// @return -1, 0, or +1 | ||
| static inline int sgn64(int64_t x) FUNC_ATTR_CONST | ||
| /// @return signed representation of the given time value |
There was a problem hiding this comment.
Sorry for the extremely delayed review. I think the CL is good, but I had one more thought.
Being exposed to Go a lot made me think that I would solve it with two distinct types:
- A type for points in time (proftime_T).
- A type for the subtraction of two points in time (duration_T or something else). This could be unsigned for negative durations.
profile_subwould take twoproftime_T's and return aduration_T.
(It's been a while, but I think C's automatic type conversion might make accidentally assigning one to the other far too easy, and it's not worth defining two types.)
At first sight, they're semantically quite different. One is an absolute point in time (though we don't know the starting point), and the other is a duration.
On second look, there's also something to be said for keeping them as the same type. Essentially, proftime_T (as defined above) is the duration since the processor was started. When subtracting two proftime_T's, it's just rebasing the starting point to the second operand.
I guess what I'm trying to say is: the way it is right now is IMHO a decent tradeoff, as C won't provide too much protection either way (Go would complain if one doesn't cast, even for the same underlying type).
The only thing I'd change is to document that profile_signed is meant to be used proftime_Ts that are (perhaps indirectly) the result of profile_sub calls.
The comment itself is not that important, maybe if you ever pass over the code again.
There was a problem hiding this comment.
Thanks. I will update the comment.
Essentially, proftime_T (as defined above) is the duration since the processor was started.
That was my conclusion as well, though I didn't inspect every call site in the codebase.
For "backwards" duration, reltimefloat() should return negative value like its counterpart reltimestr(). ref bab24a8 ref 06af88c ref neovim#10521 fix neovim#10452
For "backwards" duration, reltimefloat() should return negative value like its counterpart reltimestr(). ref bab24a8 ref 06af88c ref neovim#10521 fix neovim#10452
- reltimestr(): Produce negative value by comparing the unsigned proftime_T value to INT64_MAX. neovim#10452 (comment) 1. The interfaces of nearly all platforms return uint64_t. INT64_MAX is only half of that. 2. Low-level interfaces like this typically define that there is no fixed starting point. The only guarantees are that it's (a) monotonically increasing at a rate that (b) matches real time. ref 06af88c fix neovim#10452
For "backwards" duration, reltimefloat() should return negative value like its counterpart reltimestr(). ref bab24a8 ref 06af88c ref neovim#10521 fix neovim#10452
#10452 (comment)
ref 06af88c
fix #10452
cc @aktau