Some 2.2 performance improvements. #1897
Conversation
|
Thanks for working on this. It may be helpful to set a specific time in the release cycle before the release candidate stage to focus on performance improvements driven by profiling. |
I agree.. I'm only working on this now because while Mixxx 2.2 is usable with 2 decks, when you go up to 4 decks with 4 spinnies, it's unable to perform smoothly on ridiculously high-spec hardware (4-core 2017 macbook pro) |
|
We can update the desired rate to 2x while keeping the fix, but note that
that increses CPU usage for me a fair bit on macOS (rendering text at 20
fps isn't cheap, and causes the rest of the GUI to be repainted to some
extent). Maybe increasing the rate by 1.5x instead of 2 would be ok?
…On Tue, Nov 13, 2018, 11:26 PM Daniel Schürmann ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/engine/enginebuffer.cpp
<#1897 (comment)>:
> @@ -1235,7 +1235,7 @@ void EngineBuffer::updateIndicators(double speed, int iBufferSize) {
// Update indicators that are only updated after every
// sampleRate/kiUpdateRate samples processed. (e.g. playposSlider)
- if (m_iSamplesCalculated > (m_pSampleRate->get() / kiPlaypositionUpdateRate)) {
+ if (m_iSamplesCalculated > (kSamplesPerFrame * m_pSampleRate->get() / kiPlaypositionUpdateRate)) {
This fixes a documentation error, but effectively slows down the update
rate significant. Is this necessary to fix this bug?
Before we had a 50 ms rate, now we have 100 ms and 400 ms for BPM. The old
values have just worked. Does it really help? Else I prefere only fix the
code documentation.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1897 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABnnzzBmO-xkH9x9lKo7Stzob81gPd2ks5uu8WwgaJpZM4YagWS>
.
|
|
Any update rate works for me that solves you issue and does not look odd. |
|
LGTM |
|
Hmm, I tested the commit before 84054a9 and CPU usage with one deck playing was the same as without this branch (~60%). I'm amazed what a big difference that change makes. I wonder if there are other relatively low hanging fruit for optimization like that where we are emitting signals between threads with a high frequency... |
|
@rryan what do you think about merging this? |
|
I feel they're all pretty safe except for 3205687, given the issue with Windows. |
|
I agree. Can you rebase to remove that commit from this branch? |
This showed up in profiling as a hotspot after disabling waveforms.
WSpinny widgets are QGLWidgets with autoBufferSwap enabled. This means that after they are painted, they automatically run swapBuffers(). This can block the main thread until a vsync occurs, and generally interferes with the VSyncThread's efforts to render all the waveform widgets in one step then swap them all at once.
The constants suggest the update rate was 10 Hz, but due to a sample vs. frames difference it is actually 20 Hz. This fixes the constant values without changing behavior.
Fixes LP Bug #1801636.
GuiTick is delivering more and more render signals to widgets in the main thread, and when emitted from the VSyncThread, we're putting a heavy load on the Qt event queue. When these signals are emitted from WaveformWidgetRenderer::render(), they are emitted from the main thread, and so can be delivered directly rather than queued.
At 20 Hz, the GUI spends a lot of time re-drawing the WNumberPos widgets (which can trigger re-renders of the whole UI, since their parents need to be re-drawn).
|
Rebased to remove 3205687. |
…aveform widgets. Fixes a bug where software-renderered waveform types do not work introduced in PR mixxxdj#1897.
This is a grab bag of 2.2 performance tweaks, aimed at further improving performance on macOS after #1809. These were mostly discovered with a profiler while trying to get Mixxx to use less than 80% of a 4-core 2017 macbook pro when sitting idle ;).