GDType: Consolidate integer constants in property map - #122727
Open
DeeJayLSP wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ClassDBtoGDType. AccelerateObjectproperty access 1.6x #122596What problem(s) does this PR solve?
#112596 introduced property maps in
GDType, which made integer constants exist both in a dedicated integer constant map and the property map.Like how it's done with setgets, it should be possible to get the property map, validate that the type is integer constant, then get the integer from its payload.
Memory usage test
scons target=editor linker=mold dev_build=yes optimize=debug debug_symbols=yes werror=yes tests=yesTest script:
On a simple project that just prints
OS.get_static_memory_usage()then quits, I got the following from running in headless mode:masterEffectively a reduction of 2.46 MiB.
Additional information
The aforementioned PR mentions the new maps added over 20MB in RAM usage. This is just my attempt to mitigate part of it, but it could be superseded as soon as someone comes with a better/bigger solution to the problem.