Skip to content

Conversation

@kevinherron
Copy link
Contributor

@kevinherron kevinherron commented Jan 5, 2026

What problem(s) was I solving?

The addMonitoredItem method in OpcUaSubscription had two performance and correctness issues:

  1. O(n) lookup: Used monitoredItems.containsValue(item) which scans the entire map on every call. For subscriptions with thousands of monitored items, this becomes a significant bottleneck.

  2. Thread safety: The itemsToDelete collection was an ArrayList which is not thread-safe and has O(n) removal complexity.

What user-facing changes did I ship?

No API changes. This is a performance optimization that improves scalability for subscriptions with large numbers of monitored items.

How I implemented it

  1. Changed itemsToDelete data structure: Replaced ArrayList<OpcUaMonitoredItem> with a Set backed by ConcurrentHashMap via Collections.newSetFromMap(). This provides:

    • Thread-safe operations without external synchronization
    • O(1) removal instead of O(n)
  2. Rewrote addMonitoredItem logic: Instead of scanning the map values, the method now:

    • Checks if the item already has a client handle
    • If yes, performs O(1) map lookup by handle to check if already present
    • If the item is pending deletion, restores it to the map
    • If it's a brand-new item, assigns a new client handle and adds it

The new implementation explicitly handles three cases:

  • Item already in the map (no-op, O(1) check)
  • Item pending deletion (restore to map)
  • Brand-new item (assign handle and add)

How to verify it

Manual Testing

  • Run the integration tests to ensure subscription behavior is unchanged
  • Test with a subscription containing many monitored items to verify performance improvement

Description for the changelog

Improved OpcUaSubscription.addMonitoredItem performance from O(n) to O(1) by using direct map lookup instead of value scanning, and made the pending deletion collection thread-safe.

- 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
@kevinherron kevinherron marked this pull request as draft January 5, 2026 22:40
@kevinherron kevinherron marked this pull request as ready for review January 5, 2026 22:40
@kevinherron kevinherron closed this Jan 5, 2026
@kevinherron kevinherron reopened this Jan 5, 2026
@kevinherron kevinherron closed this Jan 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants