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

[IM] Missing support for AttributeAccessInterface to report supported attributes #31405

Open
tcarmelveilleux opened this issue Jan 13, 2024 · 0 comments

Comments

@tcarmelveilleux
Copy link
Contributor

CommandHandlerInterface supports listing the supported commands and using that knowledge within dispatch to properly handle the code-backed logic of a cluster for only available commands.

We are missing the equivalent for AttributeAccessInterface, which would allow dynamic reporting of supported attributes, such as for cases like NetworkCommissioningCluster that may have a set of attributes that is only known at runtime.

FYI @bzbarsky-apple

@tcarmelveilleux tcarmelveilleux added this to the 1.3 TE2 milestone Jan 13, 2024
@tcarmelveilleux tcarmelveilleux self-assigned this Jan 13, 2024
tcarmelveilleux added a commit to tcarmelveilleux/connectedhomeip that referenced this issue Mar 4, 2024
- Running TC_DeviceBasicComposition test caused, for a single Wildcard read,
  44622 iterations through the AttributeAccessInterface (AAI) lookup loop. This
  is significant runtime cost on embedded devices, when a wildcard subscription
  is established or when trying to iterate over changes of attributes.
- Instrumented analysis showed that significant locality exists within
  the lookup, which could be exploited by a cache.
- This PR introduces a cache, and its use, in AAI lookups. On same workload as
  the initial investigation, the total number of iterations of the costly loop
  went from 44622 --> 4864, a factor of almost 10, for very little additional
  RAM usage (single entry in cache).

Issue project-chip#31405

Testing done:
- Added unit tests
- Integration tests still pass
- TC_BasicDeviceComposition still succeeds
mergify bot pushed a commit that referenced this issue Mar 4, 2024
* Introduce AttributeAccessInterface lookup cache

- Running TC_DeviceBasicComposition test caused, for a single Wildcard read,
  44622 iterations through the AttributeAccessInterface (AAI) lookup loop. This
  is significant runtime cost on embedded devices, when a wildcard subscription
  is established or when trying to iterate over changes of attributes.
- Instrumented analysis showed that significant locality exists within
  the lookup, which could be exploited by a cache.
- This PR introduces a cache, and its use, in AAI lookups. On same workload as
  the initial investigation, the total number of iterations of the costly loop
  went from 44622 --> 4864, a factor of almost 10, for very little additional
  RAM usage (single entry in cache).

Issue #31405

Testing done:
- Added unit tests
- Integration tests still pass
- TC_BasicDeviceComposition still succeeds

* Restyled by clang-format

* Appease the Gods of Lint with a BUILD.gn entry

* Address review comments by simplifying interface

* Restyled by clang-format

* Update src/app/AttributeAccessInterfaceCache.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
erwinpan1 pushed a commit to erwinpan1/connectedhomeip that referenced this issue Mar 7, 2024
* Introduce AttributeAccessInterface lookup cache

- Running TC_DeviceBasicComposition test caused, for a single Wildcard read,
  44622 iterations through the AttributeAccessInterface (AAI) lookup loop. This
  is significant runtime cost on embedded devices, when a wildcard subscription
  is established or when trying to iterate over changes of attributes.
- Instrumented analysis showed that significant locality exists within
  the lookup, which could be exploited by a cache.
- This PR introduces a cache, and its use, in AAI lookups. On same workload as
  the initial investigation, the total number of iterations of the costly loop
  went from 44622 --> 4864, a factor of almost 10, for very little additional
  RAM usage (single entry in cache).

Issue project-chip#31405

Testing done:
- Added unit tests
- Integration tests still pass
- TC_BasicDeviceComposition still succeeds

* Restyled by clang-format

* Appease the Gods of Lint with a BUILD.gn entry

* Address review comments by simplifying interface

* Restyled by clang-format

* Update src/app/AttributeAccessInterfaceCache.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
huangxuyong pushed a commit to huangxuyong/connectedhomeip that referenced this issue Mar 19, 2024
* Introduce AttributeAccessInterface lookup cache

- Running TC_DeviceBasicComposition test caused, for a single Wildcard read,
  44622 iterations through the AttributeAccessInterface (AAI) lookup loop. This
  is significant runtime cost on embedded devices, when a wildcard subscription
  is established or when trying to iterate over changes of attributes.
- Instrumented analysis showed that significant locality exists within
  the lookup, which could be exploited by a cache.
- This PR introduces a cache, and its use, in AAI lookups. On same workload as
  the initial investigation, the total number of iterations of the costly loop
  went from 44622 --> 4864, a factor of almost 10, for very little additional
  RAM usage (single entry in cache).

Issue project-chip#31405

Testing done:
- Added unit tests
- Integration tests still pass
- TC_BasicDeviceComposition still succeeds

* Restyled by clang-format

* Appease the Gods of Lint with a BUILD.gn entry

* Address review comments by simplifying interface

* Restyled by clang-format

* Update src/app/AttributeAccessInterfaceCache.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

1 participant