Skip to content
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

Add wxXmlResource::XRCIDCount() to determine the number of known XRCIDs #24542

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joro75
Copy link
Contributor

@joro75 joro75 commented May 18, 2024

Added the helper function XRCIDCount() to wxXmlResource to determine the number of XRCID that are present in the XRC XML Resource.
This can be helpful to detemine and investigate the number of XRCID in the internal cache. Also tests are added to check the implementation, and other tests that are now possible to verify the internal behaviour of the XRCID functions.

@joro75
Copy link
Contributor Author

joro75 commented May 18, 2024

Some additional information regarding this suggestion: We have an application with over 300 XRC files (some with several dialogs/panels), and at this moment we have no idea, how many XRCID's are used, and if this is a problem for the internal XRCID cache. This addition is a general addition, and does allow us to get some more information if the XRCID_Records should be increased/optimized.

@vadz
Copy link
Contributor

vadz commented May 19, 2024

Thanks for the explanation but I'm still not entirely sure what are you going to do with this information? I.e. suppose you find that 12,345 IDs are used — now what? Sorry if it seems like a stupid question, but I just don't see how could the result of the new function be useful for anything... If there is a problem with XRC IDs hash performance, wouldn't it be better to explore replacing the extremely simplistic hash table used there now with something more performant?

Also, any new function should be documented in interface/wx/xmlres.h, but at the moment this is secondary as I'm not sure at all we really want to add it. Please let me know if I'm missing something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants