Skip to content

Commit a512cb1

Browse files
committed
Optimize addMonitoredItem with O(1) lookup and fix concurrency
- Change itemsToDelete from ArrayList to ConcurrentHashMap-backed Set for thread-safe operations and O(1) removal - Rewrite addMonitoredItem to use direct map lookup instead of containsValue O(n) scan - Handle three cases explicitly: item already in map, item pending deletion, and brand-new item
1 parent a46410f commit a512cb1

File tree

1 file changed

+23
-10
lines changed

1 file changed

+23
-10
lines changed

opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/subscriptions/OpcUaSubscription.java

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ public class OpcUaSubscription {
6868
* MonitoredItems that have been removed from the Subscription and are pending deletion on the
6969
* Server.
7070
*/
71-
private final List<OpcUaMonitoredItem> itemsToDelete = new ArrayList<>();
71+
private final Set<OpcUaMonitoredItem> itemsToDelete =
72+
Collections.newSetFromMap(new ConcurrentHashMap<>());
7273

7374
private final ClientHandleSequence clientHandleSequence =
7475
new ClientHandleSequence(monitoredItems::containsKey);
@@ -313,18 +314,30 @@ public CompletionStage<Unit> deleteAsync() {
313314
* @throws UaRuntimeException if the Subscription has not been created yet.
314315
*/
315316
public void addMonitoredItem(OpcUaMonitoredItem item) {
316-
if (!monitoredItems.containsValue(item)) {
317+
Optional<UInteger> existingHandle = item.getClientHandle();
318+
319+
if (existingHandle.isPresent()) {
320+
UInteger handle = existingHandle.get();
321+
322+
// O(1) check: if this item is already in the map, nothing to do
323+
if (monitoredItems.get(handle) == item) {
324+
return;
325+
}
326+
327+
// Item has a handle but isn't in map - check if pending deletion
317328
if (itemsToDelete.remove(item)) {
318-
monitoredItems.put(item.getClientHandle().orElseThrow(), item);
319-
} else {
320-
UInteger clientHandle = clientHandleSequence.nextClientHandle();
321-
item.setClientHandle(clientHandle);
329+
monitoredItems.put(handle, item);
330+
}
331+
// else: item has a handle from a different context, ignore
332+
} else {
333+
// Brand-new item with no handle
334+
UInteger clientHandle = clientHandleSequence.nextClientHandle();
335+
item.setClientHandle(clientHandle);
322336

323-
monitoredItems.put(clientHandle, item);
337+
monitoredItems.put(clientHandle, item);
324338

325-
if (syncState != SyncState.INITIAL) {
326-
syncState = SyncState.UNSYNCHRONIZED;
327-
}
339+
if (syncState != SyncState.INITIAL) {
340+
syncState = SyncState.UNSYNCHRONIZED;
328341
}
329342
}
330343
}

0 commit comments

Comments
 (0)