The changeset has been generated automatically through:
./mach static-analysis check --output ./clang-tidy-bugprone.json --format json --fix
followed by manual adjustment when needed or when the auto-fixer bailed out.
This paves the way for Bug 2023527
Differential Revision: https://phabricator.services.mozilla.com/D287941
The changeset has been generated automatically through:
./mach static-analysis check --output ./clang-tidy-bugprone.json --format json --fix
followed by manual adjustment when needed or when the auto-fixer bailed out.
This paves the way for Bug 2023527
Differential Revision: https://phabricator.services.mozilla.com/D287941
`mozilla::IsPowerOfTwo` is used in more code than the previous functions, so split
up the updates in three parts. This part updates non-SpiderMonkey code to use
`std::has_single_bit`. The next part updates SpiderMonkey code and the last part
finally removes `mozilla::IsPowerOfTwo`.
Differential Revision: https://phabricator.services.mozilla.com/D286320
This reverts commit eb884a9f65.
Revert "Bug 2020608 - Part 16: Use std::bit_ceil for RoundUpPow2. r=sergesanspaille"
This reverts commit 35a310aa8a.
Revert "Bug 2020608 - Part 15: Require unsigned integral types for CeilingLog2 and FindMostSignificantBit. r=sergesanspaille,nbp"
This reverts commit f682901ca2.
Revert "Bug 2020608 - Part 14: Remove mozilla::IsPowerOfTwo. r=sergesanspaille"
This reverts commit b998e4bd39.
Revert "Bug 2020608 - Part 13: Replace IsPowerOfTwo with std::has_single_bit in js/. r=sergesanspaille"
This reverts commit 00db5e1f32.
Revert "Bug 2020608 - Part 12: Replace IsPowerOfTwo with std::has_single_bit outside of js/. r=sergesanspaille,profiler-reviewers,win-reviewers,gstoll,canaltinova"
This reverts commit cec056f5d6.
Revert "Bug 2020608 - Part 11: Simplify PowerOfTwo. r=sergesanspaille,profiler-reviewers,canaltinova"
This reverts commit a2ee5b9053.
Revert "Bug 2020608 - Part 10: Prefer std::bit_width in MathAlgorithms.h. r=sergesanspaille"
This reverts commit 89d966bb97.
Revert "Bug 2020608 - Part 9: Prefer std::bit_width instead of std::countl_zero in js/src. r=sergesanspaille"
This reverts commit da9a43075a.
Revert "Bug 2020608 - Part 8: Replace CountTrailingZeroes with std::countr_zero. r=sergesanspaille"
This reverts commit d77ca273cb.
Revert "Bug 2020608 - Part 7: Replace CountLeadingZeroes32 with std::countl_zero or std::countl_one. r=sergesanspaille,nbp,profiler-reviewers,canaltinova"
This reverts commit 94929ae729.
Revert "Bug 2020608 - Part 6: Replace CountLeadingZeroes64 with std::countl_zero. r=sergesanspaille,profiler-reviewers,canaltinova"
This reverts commit 9b428168df.
Revert "Bug 2020608 - Part 5: Replace CountTrailingZeroes32 with std::countr_zero. r=sergesanspaille,manuel"
This reverts commit d3e77036b2.
Revert "Bug 2020608 - Part 4: Replace CountTrailingZeroes64 with std::countr_zero or std::countr_one. r=sergesanspaille"
This reverts commit 6a7bcee10e.
Revert "Bug 2020608 - Part 3: Replace CountPopulation32 with std::popcount. r=sergesanspaille"
This reverts commit e7b204177b.
Revert "Bug 2020608 - Part 2: Replace CountPopulation64 with std::popcount. r=sergesanspaille"
This reverts commit 1d6f98a622.
Revert "Bug 2020608 - Part 1: Add <bit> C++20 definitions to std.py. r=sergesanspaille"
This reverts commit 3ee4c6dcab.
`mozilla::IsPowerOfTwo` is used in more code than the previous functions, so split
up the updates in three parts. This part updates non-SpiderMonkey code to use
`std::has_single_bit`. The next part updates SpiderMonkey code and the last part
finally removes `mozilla::IsPowerOfTwo`.
Differential Revision: https://phabricator.services.mozilla.com/D286320
This reverts commit 64f1200593.
Revert "Bug 1881627: HiResWindowsMonitor switches between hi- and lo-res timers depending on the tolerance of the wait. r=smaug,gstoll"
This reverts commit b26c5e86c4.
Revert "Bug 1881627: Compute wait delay tolerance in TimerThread r=smaug"
This reverts commit 35fff48605.
Revert "Bug 1881627: Implemented HiResWindowsMonitor and used it in TimerThread (on Windows) r=smaug"
This reverts commit 9a0f2b30a0.
Revert "Bug 1881627: Templated *MonitorAuto* r=smaug"
This reverts commit 695384e7e0.
Revert "Bug 1881627: Got rid of WindowsTimerFrequencyManager and related machinery r=smaug"
This reverts commit 2f1747630b.
Bug 1995254 fixed the underlying problem that made this assertion trip occasionally in CI.
Together we can remove an already unused DEBUG only function in TimeStamp that was added in bug 1829983 when investigating an intermittent faliure on Windows with this assertion.
Differential Revision: https://phabricator.services.mozilla.com/D272995
Short living event targets (like SharedThreadPool or TaskQueue or even dying threads)
must never be released while we hold TimerThread's monitor locked. PostTimerEvent needs
to keep a strong reference to the target before unlocking and must release that
reference before re-acquiring the lock. The same goes for nsTimerEvent.
Differential Revision: https://phabricator.services.mozilla.com/D257966
This also drops the category argument. The category string was confusing
because it ends up a field of the marker and not the actual category which is
an enum. Having the category as a field isn't really helpful.
This also sets us up for unifying AUTO_PROFILER_MARKER and
AUTO_PROFILER_MARKER_UNTYPED which become basically the same.
Differential Revision: https://phabricator.services.mozilla.com/D269992
This also drops the category argument. The category string was confusing
because it ends up a field of the marker and not the actual category which is
an enum. Having the category as a field isn't really helpful.
This also sets us up for unifying AUTO_PROFILER_MARKER and
AUTO_PROFILER_MARKER_UNTYPED which become basically the same.
Differential Revision: https://phabricator.services.mozilla.com/D269992
This also drops the category argument. The category string was confusing
because it ends up a field of the marker and not the actual category which is
an enum. Having the category as a field isn't really helpful.
This also sets us up for unifying AUTO_PROFILER_MARKER and
AUTO_PROFILER_MARKER_UNTYPED which become basically the same.
Differential Revision: https://phabricator.services.mozilla.com/D269992
This also drops the category argument. The category string was confusing
because it ends up a field of the marker and not the actual category which is
an enum. Having the category as a field isn't really helpful.
This also sets us up for unifying AUTO_PROFILER_MARKER and
AUTO_PROFILER_MARKER_UNTYPED which become basically the same.
Differential Revision: https://phabricator.services.mozilla.com/D269992
This avoids the small overhead and potential for bugs from the previous
approach of assuming all `const char*` arguments passed to `nsITimer`
initializers are string literals, by instead requiring the argument to be a
`const nsACString&`. This involves changing all callsites to pass a `""_ns`
string literal down instead of a bare string literal.
It appears that while there were currently no buggy callers which
would've provided a dynamic string, there was at least one piece of code
(nsRepeatService::Start) which did pass down a raw char* which may be
dynamically allocated (all callers pass down `nsLiteralCString` so we're
actually OK here right now, but this was a footgun).
Differential Revision: https://phabricator.services.mozilla.com/D261032
This avoids risks which can be caused by reading the name for things
such as the profiler. By fetching the name from the callback when the
nsITimer is initialized, we avoid potential race conditions, and also
avoid the potential for re-entrancy when reading the name from a timer.
As part of this change, timer names are now always stored as a
`nsCString` (as they may be dynamic due to JS or custom nsINamed
implementations). The `const char*` arguments passed to function and closure
based constructors are assumed to be string literals, and passed to
`AssignLiteral`.
Differential Revision: https://phabricator.services.mozilla.com/D261031
This reverts commit 8c929cc018.
Revert "Bug 1979013 - Part 2: Require nsITimer API callers to provide a nsACString, r=xpcom-reviewers,media-playback-reviewers,webrtc-reviewers,profiler-reviewers,dom-storage-reviewers,toolkit-telemetry-reviewers,firefox-style-system-reviewers,mstange,emz,bwc,janv,emilio,padenot"
This reverts commit e01c0a6a32.
Revert "Bug 1979013 - Part 1: Eagerly fetch name for timer callbacks, r=xpcom-reviewers,mccr8"
This reverts commit 9f8e73c163.
This avoids the small overhead and potential for bugs from the previous
approach of assuming all `const char*` arguments passed to `nsITimer`
initializers are string literals, by instead requiring the argument to be a
`const nsACString&`. This involves changing all callsites to pass a `""_ns`
string literal down instead of a bare string literal.
It appears that while there were currently no buggy callers which
would've provided a dynamic string, there was at least one piece of code
(nsRepeatService::Start) which did pass down a raw char* which may be
dynamically allocated (all callers pass down `nsLiteralCString` so we're
actually OK here right now, but this was a footgun).
Differential Revision: https://phabricator.services.mozilla.com/D261032
This avoids risks which can be caused by reading the name for things
such as the profiler. By fetching the name from the callback when the
nsITimer is initialized, we avoid potential race conditions, and also
avoid the potential for re-entrancy when reading the name from a timer.
As part of this change, timer names are now always stored as a
`nsCString` (as they may be dynamic due to JS or custom nsINamed
implementations). The `const char*` arguments passed to function and closure
based constructors are assumed to be string literals, and passed to
`AssignLiteral`.
Differential Revision: https://phabricator.services.mozilla.com/D261031
This changes a lot of places, but it's essentially just find and
replace. We had to change this old marker API, because the new API
uses this one to serialize the marker schema fields.
This patch removes the AddKey{Label,}FormatSearchable methods and
switches to use the original AddKey{Label,}Format methods with an
optional parameter instead to make sure that we can pass multiple flags
instead of just Searchable.
For Rust, since we can't have default argument values, we had to create
a new method for them.
Differential Revision: https://phabricator.services.mozilla.com/D260465
The linear scan for the insertion point can be replaced by a binary search.
All the other logic of finding empty slots or removing entries remains unchanged by this.
Resulting complexity:
- Changed: AddTimer becomes minimum O(log[n]) (due to the lower boundary of the binary search) to maximum O(n) depending on the luck we have with finding empty slots.
- Changed: RemoveTimer becomes O(log(n)) always (for the binary search) as we postpone the removal of canceled timers to happen in the Run loop or through re-use.
- Extracting the next timer to fire on the TimerThread remains always O(n) (but happens on the timer thread, where it might disturb less).
- Removing the leading canceled timers remains always O(n) (but happens on the timer thread, where it might disturb less).
Differential Revision: https://phabricator.services.mozilla.com/D249902
Entry's lifecycle was strictly bound to (Set)IsInTimerThread. It can become a simple struct which will ease its handling in sorted arrays (or sets).
We simply make explicit calls to SetIsInTimerThread where the add/remove occurs.
AddTimerInternal's many if clauses can be re-arranged in a simpler loop, the way of re-using empty slots should be unchanged by this. This enables us also to not need the ctor for canceled timers in Entry.
Note also that the single loop with swap is faster than scanning first for empty slots and in the not unlikely case of no empty slots moving the tail of timers then.
Differential Revision: https://phabricator.services.mozilla.com/D249901
This will help us later to craft stable and unique sort conditions for TimerThread::mTimers, enabling binary search on the array.
Differential Revision: https://phabricator.services.mozilla.com/D249899
Given TimerThread::Entry contains a copy of the timeout, we can use that everywhere we have no lock on nsTimerImpl::mMutex, relying on the invariant that the timeout must not change while the timer is in the TimerThread.
Doing so enables us to annotate it and have stricter checks.
Note that one unsafe, DEBUG-only check remains but it is explicitly annotated as such. TSAN will tell if we are safe enough.
Differential Revision: https://phabricator.services.mozilla.com/D249898
The original intent of this DEBUG only function was to verify our book-keeping.
However, it prevents us from applying strict thread-safety checks on nsTimerImpl::mTimeout.
Given we are going to change our book-keeping up the stack, we can just remove it but:
- add an assertion directly before PostTimerEvent if the timeout remained unchanged
- add placeholder comments where to check for the correct sort order.
Differential Revision: https://phabricator.services.mozilla.com/D249897
The linear scan for the insertion point can be replaced by a binary search.
All the other logic of finding empty slots or removing entries remains unchanged by this.
Resulting complexity:
- Changed: AddTimer becomes minimum O(log[n]) (due to the lower boundary of the binary search) to maximum O(n) depending on the luck we have with finding empty slots.
- Changed: RemoveTimer becomes O(log(n)) always (for the binary search) as we postpone the removal of canceled timers to happen in the Run loop or through re-use.
- Extracting the next timer to fire on the TimerThread remains always O(n) (but happens on the timer thread, where it might disturb less).
- Removing the leading canceled timers remains always O(n) (but happens on the timer thread, where it might disturb less).
Differential Revision: https://phabricator.services.mozilla.com/D249902
Entry's lifecycle was strictly bound to (Set)IsInTimerThread. It can become a simple struct which will ease its handling in sorted arrays (or sets).
We simply make explicit calls to SetIsInTimerThread where the add/remove occurs.
AddTimerInternal's many if clauses can be re-arranged in a simpler loop, the way of re-using empty slots should be unchanged by this. This enables us also to not need the ctor for canceled timers in Entry.
Note also that the single loop with swap is faster than scanning first for empty slots and in the not unlikely case of no empty slots moving the tail of timers then.
Differential Revision: https://phabricator.services.mozilla.com/D249901
velzie