Skip to content

Refactor MappingInfo class to use XML stream parser instead of full XmlParse#15636

Merged
daschuer merged 8 commits into
mixxxdj:mainfrom
JoergAtGithub:mappinginfo1
Nov 23, 2025
Merged

Refactor MappingInfo class to use XML stream parser instead of full XmlParse#15636
daschuer merged 8 commits into
mixxxdj:mainfrom
JoergAtGithub:mappinginfo1

Conversation

@JoergAtGithub
Copy link
Copy Markdown
Member

Mixxx parses all XML controller mappings at every startup, but only the tiny info section is needed here.
This PR replace the parsing of the full XML file, by stream reading which is stopped once the info section is read. The info section is usually at the beginning of an XML mapping file.
On my system this PR saves ~200ms startup time.

Copy link
Copy Markdown
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR. I have suggested some improvements.

Comment thread src/controllers/controllermappinginfo.h Outdated
Comment thread src/controllers/controllermappinginfo.h Outdated
Comment thread src/controllers/controllermappinginfo.cpp Outdated
Comment thread src/controllers/controllermappinginfo.cpp Outdated
Comment thread src/controllers/controllermappinginfo.cpp Outdated
Comment thread src/controllers/controllermappinginfo.cpp Outdated
Comment thread src/controllers/controllermappinginfo.cpp Outdated
Comment thread src/controllers/controllermappinginfo.cpp Outdated
Copy link
Copy Markdown
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with all the changes proposed by Daniel. LGTM to me apart from those.

performance and consistency in string comparisons and attribute
lookups. Switched from `QString` to `QStringView` for lightweight,
non-owning string handling in `xmlElementName` and `protocol`
variables, reducing memory allocations.
Comment thread src/controllers/controllermappinginfo.cpp Outdated
Comment thread src/controllers/controllermappinginfo.cpp Outdated
Copy link
Copy Markdown
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. waiting for @daschuer

Copy link
Copy Markdown
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you.
Is this ready for merge now?

@JoergAtGithub
Copy link
Copy Markdown
Member Author

Yes!

@daschuer daschuer merged commit 8060afd into mixxxdj:main Nov 23, 2025
15 checks passed
@JoergAtGithub JoergAtGithub deleted the mappinginfo1 branch November 23, 2025 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants