[SERVER-47709] Add function to coerce numbers to Duration types Created: 22/Apr/20 Updated: 29/Oct/23 Resolved: 22/Jan/21 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 4.9.0 |
| Type: | New Feature | Priority: | Major - P3 |
| Reporter: | Benjamin Caimano (Inactive) | Assignee: | Billy Donahue |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | servicearch-wfbf-day | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Backwards Compatibility: | Fully Compatible |
| Sprint: | Service arch 2020-05-04, Service arch 2020-05-18, Service arch 2020-12-28, Service Arch 2021-02-08 |
| Participants: |
| Description |
|
Our Duration types expect signed integral types upon construction. However, occasionally, our time data is in double or unsigned integer form. We should make functions to intentionally coerce these numbers into durations. |
| Comments |
| Comment by Billy Donahue [ 22/Jan/21 ] | ||||||||||||||
|
Added deduceChronoDuration function. To make full use of it we would need some fixes to mongo::duration_cast. But we can rely on a 2-step duration_cast chain in the meantime. | ||||||||||||||
| Comment by Githook User [ 22/Jan/21 ] | ||||||||||||||
|
Author: {'name': 'Billy Donahue', 'email': 'billy.donahue@mongodb.com', 'username': 'BillyDonahue'}Message: | ||||||||||||||
| Comment by Billy Donahue [ 19/Jan/21 ] | ||||||||||||||
|
Code review with JUST the deduceChronoDuration function and its tests. | ||||||||||||||
| Comment by Billy Donahue [ 09/Dec/20 ] | ||||||||||||||
|
I've resurrected and rebased my old branch in which I got this in pretty good shape a while ago. It would be a big improvement for mongo::Duration. It's almost ready to review but we will have to think of more really evil torture tests for it. I've self-bikeshedded to deduceChronoDuration. These produce std::chrono<Rep, Per> with the specified period but a deduced Rep that matches the numeric argument type.
Once you have these, duration_cast can, with maximal precision, do a correct conversion. PoC: | ||||||||||||||
| Comment by Billy Donahue [ 08/May/20 ] | ||||||||||||||
|
Our mongo::Duration<Period> types can't use arbitrary Rep like std::chrono::duration<Rep,Period> can. But we do have a mongo::duration_cast that accepts any std::chrono::duration. This is good, but it isn't general enough. It converts to our Duration rep type and then does scaling. I don't think it works well with chrono::duration<Float,Period>. I recommend two things to fix this problem.
Fix our mongo::duration_cast to be more useful and correct for any std::chrono:duration. We could just use the std::chrono::duration_cast logic, but we want our overflow and narrowing protection, so we'll have to rewrite it a little. But it's open source, and we can just go grab it and change it.
Make it easy to make a std::chrono::duration<Rep,Period> with deduced Rep.
https://gcc.godbolt.org/z/SVyy8Y I don't recommend adding members to mongo::Duration. I don't think we should combine the two steps into a convenience function. |