-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow pointer compare instead of string compare with keys, group and item #7323
Comments
Commented by: rryan Thanks for filing this but before spending any time on this bug please For sure, we should benchmark QList linear scans versus QMap and QHash
Public bug reported: If we extend the Key Api in a way, that we have every group and key
** Affects: mixxx -- Title: To manage notifications about this bug go to: |
Commented by: ywwg Due to feature freeze, this work should wait until after the 1.12 release. |
Commented by: rryan
Well, looks like I've proven myself wrong! I've been doing some profiling.
Switching to a QHash drops this to 5.9% but just shifts the bulk of the work into qHash(QString). If we solved this bug by giving every GroupHandle an incremental integer (starting at zero) then we could probably use a QVarLengthArray and index directly. I would guess the time spent doing lookups would be negligible in that case. |
Commented by: daschuer Intesting! Do you know the X11 Atom API? http://tronche.com/gui/x/xlib/window-information/XInternAtom.html We may do it quite similar.
And work inside with the Atom IDs MIxxxAtom() will look like that typedef AtomID QString*
What do you think? |
Commented by: rryan
An atom that is defined as an integer that starts at 0 and increases would allow us to use a QVarLengthArray with a more-than-expected-channels pre-allocation. Then we could have (group -> data) associative containers with essentially no cost. WDYT? |
Commented by: rryan Oh yea -- integers -> no garbage to clean up :). |
Commented by: daschuer
You are right, it is in most cases the group string, that would make the difference. The pointer version has a benefit that we can convert them back to QStrings with no costs, We can't get rid of the strings, since we need them in skins and midi scripts. Conclusion: lets introduce a Group Index! @rryan: Can you please have a look at |
Reported by: daschuer
Date: 2014-02-22T15:47:01Z
Status: Confirmed
Importance: Low
Launchpad Issue: lp1283471
If we extend the Key Api in a way, that we have every group and key string only one time in memory, we could switch to pointer compare instead of string or hash compare at many places inside Mixxx. Or we can remove some workarounds, that where introduced in hot paths to avoid the string compare.
The text was updated successfully, but these errors were encountered: