Add notes for refactoring ResourceItem #7161
Draft
+72
−13
Conversation
This file contains 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
ebaauw
reviewed
Aug 11, 2023
| BufStringCacheHandle m_strHandle; // for strings which don't fit into \c m_istr | ||
| ItemString m_istr; // internal embedded small string | ||
| deCONZ::TimeSeconds m_refreshInterval; | ||
| QString *m_str = nullptr; | ||
| const ResourceItemDescriptor *m_rid = &rInvalidItemDescriptor; | ||
| QDateTime m_lastSet; | ||
| /* | ||
| * TODO(mpi): This is included in the new struct but ultimately I think we should get rid of lastChanged | ||
| * and only mark if a change occured as flag. Most uses are lastChanged() == lastSet() |
There was a problem hiding this comment.
What about ddx rules and durationDue?
There was a problem hiding this comment.
Good point, indeed for these we might need to keep the exact change timestamp.
manup
force-pushed
the
refactor_resourceitem
branch
from
6c357fe
to
46d04b9
Compare
manup
force-pushed
the
refactor_resourceitem
branch
from
46d04b9
to
27bf451
Compare
manup
force-pushed
the
refactor_resourceitem
branch
2 times, most recently
from
6a31114
to
b050adf
Compare
The goal is to make ResourceItems as small as possible aka cache friendly and also very fast. Currently each item results in many heap allocations and is internally not very type safe. We can do a lot better here.
manup
force-pushed
the
refactor_resourceitem
branch
from
b050adf
to
1b56db3
Compare
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.
The goal is to make
ResourceItemsas small as possible aka cache friendly, and also very fast. Currently each item results in many heap allocations and is internally not very type safe. We can do a lot better here.The
QStringhas the biggest impact especially for legacy code which may do hundrets of string comparisons foe each incoming APS Indication, and timestamp to string conversions during WebSocket event generation.This PR has no code changes only comments to figure out where to go.