Summary: | Add a helper method to WTF::MachSemaphore to wait with a timeout duration | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||
Component: | Web Template Framework | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, darin, ews-watchlist, ggaren, sam, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 219586 | ||||||||||
Attachments: |
|
Description
Wenson Hsieh
2020-12-22 19:09:58 PST
Created attachment 416730 [details]
WIP
Created attachment 416823 [details]
Patch
Comment on attachment 416823 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416823&action=review > Source/WTF/wtf/cocoa/MachSemaphore.cpp:62 > +void MachSemaphore::waitWithTimeout(Seconds timeout) We call this waitFor() in WTF::Semaphore and WTF::BinarySemaphore, perhaps we should keep the names the same? > Source/WTF/wtf/cocoa/MachSemaphore.cpp:64 > + auto seconds = static_cast<unsigned>(timeout.seconds()); I think you can use timeout.secondsAs<unsigned>() here and below instead of explicitly casting yourself. Comment on attachment 416823 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416823&action=review >> Source/WTF/wtf/cocoa/MachSemaphore.cpp:62 >> +void MachSemaphore::waitWithTimeout(Seconds timeout) > > We call this waitFor() in WTF::Semaphore and WTF::BinarySemaphore, perhaps we should keep the names the same? Sounds good — changed to waitFor(). >> Source/WTF/wtf/cocoa/MachSemaphore.cpp:64 >> + auto seconds = static_cast<unsigned>(timeout.seconds()); > > I think you can use timeout.secondsAs<unsigned>() here and below instead of explicitly casting yourself. timeout.secondsAs<unsigned>() ought to work. However, timeout.nanosecondsAs<unsigned>() won't work for anything above a few seconds. I can use timeout.nanosecondsAs<int64_t>() and then subtract against the number of nanoseconds, but I'll still need a static cast to clock_res_t around the result. Created attachment 416825 [details]
Patch
Comment on attachment 416825 [details]
Patch
Thanks for the review!
Committed r271101: <https://trac.webkit.org/changeset/271101> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416825 [details]. Comment on attachment 416825 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416825&action=review > Source/WTF/wtf/cocoa/MachSemaphore.cpp:65 > + auto seconds = timeout.secondsAs<unsigned>(); > + semaphore_timedwait(m_semaphore, { seconds, static_cast<clock_res_t>(timeout.nanosecondsAs<uint64_t>() - seconds * NSEC_PER_SEC) }); Seems like the "convert Seconds to a mach_timespec_t" should be a separate function; could be useful elsewhere and a good separate concept. Simpler but related to MonotonicTime::fromMachAbsoluteTime. |