[codegen/CodegenDataModelProvider.cpp]: Move the constructor of AttributeEntry inside a closer "for" loop. #38866
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
|
PR #38866: Size comparison from 8167c5f to 7ba17d8 Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
gd-mauri
reviewed
May 12, 2025
andy31415
reviewed
May 12, 2025
|
PR #38866: Size comparison from 8167c5f to 72106ce Increases above 0.2%:
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
…y_inside_for_loop
|
PR #38866: Size comparison from dfa46da to 056b400 Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
…y_inside_for_loop
|
PR #38866: Size comparison from 70e5039 to 041021b Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
bzbarsky-apple
approved these changes
May 13, 2025
…y_inside_for_loop
|
PR #38866: Size comparison from f4d171a to 96c02e9 Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Unfortunately not yet. On the file The third line is a direct assign operation, which cannot happen when a class has a const member variable. Because the class cannot be assigned using the default assignment operator due to the const member, the compiler does not generate one. We could define our own custom assignment operator for the class |
…y_inside_for_loop
|
PR #38866: Size comparison from fdacd16 to c1137c0 Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
andy31415
approved these changes
May 14, 2025
So can we do: DataModel::AttributeFinder finder(mDataModelProvider);
std::optional<DataModel::AttributeEntry> attributeEntry = finder.Find(aPath);To use a constructor? |
Yes, we should be able to do this, using the (default) copy constructor. We probably will need to test in detail that any "shallow" copy doesn't break the unit tests, but this looks doable, yes. I will create a follow up issue addressing this. |
heinsaar
pushed a commit
to heinsaar/connectedhomeip
that referenced
this pull request
May 15, 2025
…buteEntry inside a closer "for" loop. (project-chip#38866) * Moved AttributeEntry constructor inside of "for" loop. * Using move semantics to handle object "globalListEntry". Renaming "attribute".
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.
Fixes #38769
Inside https://github.com/project-chip/connectedhomeip/blob/master/src/data-model-providers/codegen/CodegenDataModelProvider.cpp, at the function
Attributes(), we have the declaration of the variableglobalListEntryof typeDataModel::AttributeEntry. This declaration calls theAttributeEntryconstructor, with the first parameter (attributeId) set to zero by default. This value is set later inside aforloop, immediately after theglobalListEntrydeclaration.We have moved the
AttributeEntryconstructor call inside this nearforloop, since in here we already know the value of the first parameter, and we can avoid calling the constructor with unnecessary filler or garbage data.The codesize does not increase, and in fact it does decrease around ~1 KByte.
We got rid of the now redundant line:
globalListEntry.attributeId = attribute;inside the
forloop.Also, the use of
std::move()enables move semantics, which allows efficient transfer of resources (in this case, theDataModel::AttributeEntryinstance) from one object to another, avoiding unnecessary copies.Function to change
CodegenDataModelProvider::Attributes()Testing
CI checks will be run against this PR.