Node read optimization #1699
Merged
Node read optimization #1699
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
pullapprove
bot
requested review from
Apollon77,
turon,
vves and
woody-apple
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
lauckhart
force-pushed
the
efficient-node-read
branch
from
ce72ba3 to
7946e91
Compare
Apollon77
approved these changes
Feb 11, 2025
packages/node/src/node/server/TransactionalInteractionServer.ts
Outdated
Show resolved
Hide resolved
This offers a new path for performing attribute reads against the Node API. As discussed it is NOT fully wired yet so we can discuss first, but this PR should be ready for merge. There's significant new code here. Some of this is noise, which I detail below, but here are the main bits of new functionality to focus on: 1. packages/protocol/src/action/** - this is a new API for performing interactions that is designed for simplicity and performance - action/protocols.ts - these define a `ProtocolNode` JS contract for exposing a Matter data model in a form that efficiently maps to the wire protocol - action/server/AttributeResponse.ts - implements the attribute subset of a Read Request Action against ProtocolNode - action/Interactable.ts and request/* - an API I created to simplify construction of Matter actions. The goal was to make it easy to initiate Matter interactions for e.g. batch purposes, but with a more convenient JS API. Much of this is stubbed due to focus on read in this commit but I do use some of the type definitions 2. packages/node/src/node/server/ProtocolService.ts - implements ProtocolNode for a @matter/node Node 3. packages/behavior/state/managed/values/StructManager.ts - I updated struct manager to generate ID-based getters and setters. This means that where state values are associated with a Matter element that has an ID you can use `state[id]` and `state.name` interchangeably. I had done this to support attributes that are not in the schema but it also allows us to map protocol queries against state objects without performing name lookup It became apparent as I was working through this that the code was going to be too @matter/node specific unless I performed some code reorganization. Unfortunately that adds noise to the commit but I think it's worthwhile. Moved modules include: - node/src/behavior/state/transaction -> general/src/transaction - node/src/behavior/state/Val.ts -> protocol/src/action - node/src/behavior/errors.ts -> protocol/src/action - node/src/behavior/AccessControl.ts -> protocol/src/action/server Also unfortunate... I encountered a Mocha bug while developing this that indicated there were import cycles. I added a "matter-build cycles" command to detect cycles and fixed all cycles in the node package, but this ended up changing a bunch of extra files that are now hard to disentangle from the commit.
lauckhart
force-pushed
the
efficient-node-read
branch
from
5d5b6ab to
035a4e3
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.
This offers a new path for performing attribute reads against the Node API. As discussed it is NOT fully wired yet so we can discuss first, but this PR should be ready for merge.
There's significant new code here. Some of this is noise, which I detail below, but here are the main bits of new functionality to focus on:
ProtocolNodeJS contract for exposing a Matter data model in a form that efficiently maps to the wire protocolstate[id]andstate.nameinterchangeably. I had done this to support attributes that are not in the schema but it also allows us to map protocol queries against state objects without performing name lookupIt became apparent as I was working through this that the code was going to be too @matter/node specific unless I performed some code reorganization. Unfortunately that adds noise to the commit but I think it's worthwhile.
Moved modules include:
Also unfortunate... I encountered a Mocha bug while developing this that indicated there were import cycles. I added a "matter-build cycles" command to detect cycles and fixed all cycles in the node package, but this ended up changing a bunch of extra files that are now hard to disentangle from the commit.