Skip to content

Commit

Permalink
Node read optimization
Browse files Browse the repository at this point in the history
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
  a. 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
  b. action/server/AttributeResponse.ts - implements the attribute subset of a Read Request Action against ProtocolNode
  c. 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.
  • Loading branch information
lauckhart committed Feb 10, 2025
1 parent 56b2c53 commit ce72ba3
Show file tree
Hide file tree
Showing 182 changed files with 4,313 additions and 1,542 deletions.
2 changes: 1 addition & 1 deletion codegen/src/generate-forwards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ await progress.run("@project-chip/matter.js", generateProjectChipMatterjsForward

await progress.run("@matter/main", generateMatterjsMainForwards);

progress.shutdown();
progress.close();
457 changes: 271 additions & 186 deletions package-lock.json

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions packages/general/src/MatterError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,3 +223,21 @@ function fallbackFormatter(value: unknown, indents = 0) {

return formatOne(value, indents, "");
}

/**
* Indicate an asynchronous operation was canceled.
*/
export class CanceledError extends MatterError {
constructor(message = "Operation canceled", options?: ErrorOptions) {
super(message, options);
}
}

/**
* Indicates an asynchronous operation was canceled due to timeout.
*/
export class TimeoutError extends CanceledError {
constructor(message = "Operation timed out", options?: ErrorOptions) {
super(message, options);
}
}
1 change: 1 addition & 0 deletions packages/general/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ export * from "./MatterError.js";
export * from "./net/index.js";
export * from "./storage/index.js";
export * from "./time/index.js";
export * from "./transaction/index.js";
export * from "./util/index.js";
import "./polyfills/index.js";
3 changes: 3 additions & 0 deletions packages/general/src/log/Logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import { Boot } from "#util/Boot.js";
import { CancelablePromise } from "#util/Cancelable.js";
import { ImplementationError, NotImplementedError } from "../MatterError.js";
import { Time } from "../time/Time.js";
import { Bytes } from "../util/Bytes.js";
Expand Down Expand Up @@ -491,3 +492,5 @@ Boot.init(() => {
MatterHooks.loggerSetup?.(Logger);
}
});

CancelablePromise.logger = Logger.get("CancelablePromise");
2 changes: 1 addition & 1 deletion packages/general/src/time/Time.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { Boot } from "#util/Boot.js";
import { CancelablePromise } from "#util/Promises.js";
import { CancelablePromise } from "#util/Cancelable.js";
import { ImplementationError } from "../MatterError.js";
import { Diagnostic } from "../log/Diagnostic.js";
import { DiagnosticSource } from "../log/DiagnosticSource.js";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { MaybePromise } from "#general";
import { MaybePromise } from "#util/Promises.js";

/**
* Components with support for transactionality implement this interface.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { describeList, Logger } from "#general";
import { SynchronousTransactionConflictError, TransactionDeadlockError, TransactionFlowError } from "./Errors.js";
import { Logger } from "#log/Logger.js";
import { describeList } from "#util/String.js";
import { SynchronousTransactionConflictError, TransactionDeadlockError, TransactionFlowError } from "./errors.js";
import { Resource } from "./Resource.js";
import type { Transaction } from "./Transaction.js";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { TransactionFlowError } from "./Errors.js";
import { TransactionFlowError } from "./errors.js";
import type { Transaction } from "./Transaction.js";

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { MaybePromise } from "#general";
import { MaybePromise } from "#util/Promises.js";
import { Participant } from "./Participant.js";
import type { Resource } from "./Resource.js";
import { Resource } from "./Resource.js";
import { ResourceSet } from "./ResourceSet.js";
import { Status } from "./Status.js";
import { ReadOnlyTransaction, act } from "./Tx.js";

/**
* By default, Matter.js state is transactional.
* .
*
* Transactions are either shared (for reads) or exclusive (for writes). Exclusive transactions do not block shared
* transactions but state updates will not be visible until the transaction completes.
Expand All @@ -21,7 +22,7 @@ import { ReadOnlyTransaction, act } from "./Tx.js";
* can avoid this by using {@link begin} which will wait for other transactions to complete before acquiring resource
* locks.
*
* Persistence is implemented by a list of participants. Commits are two phase. If an error is thrown in phase one all
* Persistence is implemented by a list of participants. Commits are two phase. If an error throws in phase one all
* participants roll back. An error in phase 2 could result in data inconsistency as we don't have any form of retry as
* of yet.
*
Expand Down Expand Up @@ -148,8 +149,8 @@ export interface Transaction {
}

type StatusType = Status;
const StatusEnum = Status;
type ResourceType = Resource;
type ResourceSetType = ResourceSet;
type ParticipantType = Participant;

export const Transaction = {
Expand All @@ -166,12 +167,11 @@ export const Transaction = {
return act(via, actor);
},

/**
* A read-only transaction you may use without context.
*/
ReadOnly: ReadOnlyTransaction,

Status: StatusEnum,
Status,

Resource,

[Symbol.toStringTag]: "Transaction",
};
Expand All @@ -184,5 +184,7 @@ export namespace Transaction {

export type Resource = ResourceType;

export type ResourceSet = ResourceSetType;

export type Participant = ParticipantType;
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,13 @@
* SPDX-License-Identifier: Apache-2.0
*/

import {
describeList,
Diagnostic,
ImplementationError,
Logger,
MaybePromise,
Observable,
ReadOnlyError,
} from "#general";
import { StatusResponseError } from "#types";
import { FinalizationError, TransactionDestroyedError, TransactionFlowError } from "./Errors.js";
import { Diagnostic } from "#log/Diagnostic.js";
import { Logger } from "#log/Logger.js";
import { ImplementationError, ReadOnlyError } from "#MatterError.js";
import { Observable } from "#util/Observable.js";
import { MaybePromise } from "#util/Promises.js";
import { describeList } from "#util/String.js";
import { FinalizationError, TransactionDestroyedError, TransactionFlowError, UnsettledStateError } from "./errors.js";
import type { Participant } from "./Participant.js";
import type { Resource } from "./Resource.js";
import { ResourceSet } from "./ResourceSet.js";
Expand Down Expand Up @@ -99,7 +95,8 @@ export function act<T>(via: string, actor: (transaction: Transaction) => T): T {

// If actor is async, chain commit and close asynchronously
if (MaybePromise.is(actorResult)) {
isAsync = true;
// If the actor is async mark the transaction as async; this will enable reporting on lock changes
isAsync = tx.isAsync = true;
return Promise.resolve(actorResult)
.then(commitTransaction, handleTransactionError)
.finally(closeTransaction) as T;
Expand Down Expand Up @@ -139,6 +136,7 @@ class Tx implements Transaction {
#via: string;
#shared?: Observable<[]>;
#closed?: Observable<[]>;
#isAsync = false;

constructor(via: string, readonly = false) {
this.#via = Diagnostic.via(via);
Expand Down Expand Up @@ -177,6 +175,19 @@ class Tx implements Transaction {
return this.#waitingOn;
}

get isAsync(): boolean {
return this.#isAsync;
}

set isAsync(isAsync: true) {
// When the transaction is noted as async we start reporting locks. A further optimization would be to not even
// acquire locks for synchronous transactions
if (!this.#isAsync) {
this.#locksChanged(this.#resources);
}
this.#isAsync = isAsync;
}

onShared(listener: () => void, once?: boolean) {
if (this.#shared === undefined) {
this.#shared = Observable();
Expand Down Expand Up @@ -335,12 +346,7 @@ class Tx implements Transaction {
throw new TransactionFlowError("Attempted wait on a transaction that is already waiting");
}

logger.debug(
"Transaction",
this.via,
"waiting on",
describeList("and", ...[...others].map(other => other.via)),
);
logger.debug("Tx", this.via, "waiting on", describeList("and", ...[...others].map(other => other.via)));

this.#waitingOn = others;
return new Promise<void>(resolve => {
Expand Down Expand Up @@ -414,27 +420,33 @@ class Tx implements Transaction {
let cycles = 1;

const errorRollback = (error?: any) => {
logger.error(
"Rolling back",
this.via,
"due to pre-commit error:",
Diagnostic.weak(error?.message || `${error}`),
);

const result = this.#finalize(Status.RollingBack, "rolled back", () => this.#executeRollback());

if (MaybePromise.is(result)) {
return result.then(() => {
StatusResponseError.reject(error);
throw new FinalizationError("Rolled back due to pre-commit error");
throw error;
});
}

StatusResponseError.reject(error);
throw new FinalizationError("Rolled back due to pre-commit error");
throw error;
};

const nextCycle = () => {
// Guard against infinite loops
cycles++;
if (cycles > MAX_PRECOMMIT_CYCLES) {
logger.error(
`State has not settled after ${MAX_PRECOMMIT_CYCLES} pre-commit cycles which likely indicates an infinite loop`,
return errorRollback(
new UnsettledStateError(
`State has not settled after ${MAX_PRECOMMIT_CYCLES} pre-commit cycles which likely indicates an infinite loop`,
),
);
return errorRollback();
}

// Restart iteration at the first participant
Expand Down Expand Up @@ -479,7 +491,6 @@ class Tx implements Transaction {
// When an error occurs this function performs rollback then throws
const handleError = (error: any) => {
abortedDueToError = true;
logger.error(`Error pre-commit of ${participant}:`, error);
return errorRollback(error);
};

Expand Down Expand Up @@ -625,7 +636,6 @@ class Tx implements Transaction {
* Rollback logic passed to #finish.
*/
#executeRollback() {
//this.#log("rollback");
this.#status = Status.RollingBack;
let errored: undefined | Array<Participant>;
let ongoing: undefined | Array<Promise<void>>;
Expand Down Expand Up @@ -670,12 +680,8 @@ class Tx implements Transaction {
}
}

#log(...message: unknown[]) {
logger.debug("Transaction", this.#via, message);
}

#locksChanged(resources: Set<Resource>, how = "locked") {
if (!resources.size) {
if (!resources.size || !this.isAsync) {
return;
}

Expand All @@ -685,7 +691,7 @@ class Tx implements Transaction {
} else {
resourceDescription = `${resources.size} resource${resources.size === 1 ? "" : "s"}`;
}
this.#log(how, resourceDescription);
logger.debug(this.via, how, resourceDescription);
}

#assertAvailable() {
Expand All @@ -703,6 +709,9 @@ class Tx implements Transaction {
}
}

/**
* A read-only offline transaction you may use without context.
*/
export const ReadOnlyTransaction = new Tx("readonly", true);

function throwIfErrored(errored: undefined | Array<Participant>, when: string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { MatterError } from "#general";
import type { Behavior } from "../../Behavior.js";
import type { ActionContext } from "../../context/ActionContext.js";
import type { OfflineContext } from "../../context/server/OfflineContext.js";
import { MatterError } from "#MatterError.js";
import type { Transaction } from "./Transaction.js";

/**
Expand Down Expand Up @@ -36,10 +33,15 @@ export class FinalizationError extends MatterError {}
/**
* Thrown if a {@link Transaction} is accessed after it has been destroyed.
*
* If you see this error, you have probably kept a reference to a contextual object such as a {@link Behavior} after its
* {@link ActionContext} exited. You may need to create a new context using {@link OfflineContext.act}.
* If you see this error, you have probably kept a reference to a contextual object its exited. You may need to create
* a new, independent transaction context.
*
* A possible cause of this error is forgetting to use await on an async function. The context will remain open so long
* as there is an unresolved {@link Promise} it can await.
*/
export class TransactionDestroyedError extends MatterError {}

/**
* Trhown if a {@link Transaction} cannot commit because state has mutated continuously for too many pre-commit events.
*/
export class UnsettledStateError extends FinalizationError {}
8 changes: 8 additions & 0 deletions packages/general/src/transaction/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* @license
* Copyright 2022-2025 Matter.js Authors
* SPDX-License-Identifier: Apache-2.0
*/

export * from "./errors.js";
export * from "./Transaction.js";
Loading

0 comments on commit ce72ba3

Please sign in to comment.