[kevinherron]

Part 4 of 6 (#2118 to #2123) of a stack of server performance changes, found by profiling the Milo server in the o6-automation/opcua-benchmarks suite.

Problem

DefaultAccessController checks every Read, and every sampling cycle on the default read-access policy, before the address space is read. To decide, it reads NodeClass, AccessRestrictions, AccessLevel and UserAccessLevel for each node through an internal AddressSpaceManager.read. Building that internal request and the result map took six Stream pipelines per check: the pending results, the NodeIds, the attributes to read, the distinct NodeIds, a flatMap of NodeIds by attributes, and the final toMap. In a profile of 10 clients reading batches of 100 Variables, those pipelines were about a tenth of the server's CPU time.

Change

checkReadAccess and DefaultAccessControlContext.readAccessControlAttributes build the same lists with loops. The behavior is intended to be identical:

• The internal Read has the same ReadValueIds in the same order: distinct NodeIds in first-seen order, each with the attributes from ACCESS_ATTRIBUTE_ORDER that the check needs. Routing, fragment grouping, each fragment's ReadContext, and the order in which each address space reads its attributes are all unchanged.
• The result map is built exactly as Collectors.toMap(..., (a, b) -> b) built it: a default-capacity HashMap filled with merge. Equal ReadValueIds still share one entry holding the last result, and the iteration order is the same. ReadAccessCache iterates this map, so its order matters (see Follow-ups).

The other access checks (Write, Browse, Call, references, nodes) are unchanged; they did not show up in the profiles.

An approach that was tried and dropped

An earlier version of this PR skipped the internal Read for nodes held by a ManagedAddressSpace with the default read, and read the attributes straight from the node. It was about as fast as this version. Three review rounds found cases where its different execution order could turn a denial into an allowance: application AttributeFilters with side effects, nodes replaced during the check, and stateful filterRead implementations. Reproducing the generic path's order exactly would have meant re-implementing it, so this version keeps the generic path and removes only the overhead around it.

Results

Same benchmark setup as the rest of the stack: open62541 C clients, default configuration, Ryzen 9 7950X with the server on 16 CPUs, median of 6 runs.

Workload	1.2	Before this PR	With this PR	Change
async, 10 clients, batch of 100 read	57,564	118,137	129,422	+10%
sync, 1 client, batch of 100 read	6,408	9,890	10,215	+3%
async, 10 clients, scalar read	540,578	595,458	624,958	+5%
sync, 1 client, scalar read	34,382	36,604	36,902	+1%

Tests

The existing DefaultAccessControllerTest, ReadAccessCacheTest and OpcUaServerServiceSetsTest cover the decisions and pass unchanged. No new test: the change has no intended behavior difference.

Follow-ups

• ReadAccessCache collapses the access controller's results to (NodeId, AttributeId), ignoring index range and data encoding, and keeps whichever entry its iteration of the returned HashMap visits last. With stateful role answers, two ReadValueIds that differ only in index range can therefore cache different decisions depending on map order. That predates this stack. This PR keeps the map's order identical so it does not change which entry wins.

Verification

• DefaultAccessControllerTest, ReadAccessCacheTest, OpcUaServerServiceSetsTest: 50 tests pass.
• Full build with tests on the top of the stack: 5,792 tests, no failures, 4 skipped.
• Reviewed independently by GPT-6.1-Sol (Codex) in two rounds. Round 1 found that pre-sizing the map and using put changed its iteration order, which ReadAccessCache consumes; this version builds it as toMap did. Round 2 compared the two constructions against the JDK 17 and 21 Collectors sources and in 7,710 generated cases per runtime, including resize boundaries and hash collisions.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Replaces Streams with loops when building read access checks in DefaultAccessController.java.

• Read access checks build their lists and results without Streams.

_{Reviews (1) · Last reviewed commit: "Build read access checks without Streams" · Reviewed by Greptile}