question

Upvotes
Accepted
3 4 5 10

poor performance in OmmConsumerImpl.unregister(handle)

When large number of handles are registered then calls to OmmConsumerImpl.unregister(handle) become very slow.

I traced this to WlItemHandler.removeUserRequestFromClosedStream(WlRequest) which iterates over a LinkedList using an integer index, calling LinkedList.get(i) for each index. Since get(i) is O(N) on a LinkedList (each time it is called it has to walk down the list to get to that index), just this step is O(N2). If it finds a match it then calls LinkedList.remove(i) which again has to walk down the entire list to find that index and remove it.

Calling this on every handle in a set of 379233 handles took 48 minutes! 99% of the CPU time is in WlItemHandler.removeUserRequestFromClosedStream(WlRequest).

If you use an iterator you can just walk the list just once, turning the method into O(N). The performance improvement will be enormous. Basically change this

    private int removeUserRequestFromClosedStream(WlRequest wlRequest) {
        for (int i = 0; i < _userStreamIdListToRecover.size(); i++) {
            int listStreamId = _userStreamIdListToRecover.get(i);

            if (listStreamId == wlRequest.requestMsg().streamId()) {
                _userStreamIdListToRecover.remove(i);
                break;
            }
        }
    ...

to

    private int removeUserRequestFromClosedStream(WlRequest wlRequest) {
        Iterator<Integer> iter = _userStreamIdListToRecover.iterator();
        while (iter.hasNext()) {
            int listStreamId = iter.next();
            if (listStreamId == wlRequest.requestMsg().streamId()) {
                iter.remove();
                break;
            }
        }
    ...

or in this case you could even just do

   private int removeUserRequestFromClosedStream(WlRequest wlRequest) {
        _userStreamIdListToRecover.remove(wlRequest.requestMsg().streamId());
    ...

and let Java do the iterate and remove for you.

This method also has an iteration over _statusMsgDispatchList that needs the same treatment (since it does additional actions inside the loop you either have to use the first iterator form I provided, or you have check the boolean result if you use the second form and do the actions outside the loop).

There are dozens of examples of this same problem in this class, and they really all should be fixed. You might also consider a different Collection type. LinkedHashSet want to preserve order and don't want duplicates (and I assume you never want duplicates of handles) or just HashSet if you don't care about order. Both would give O(1) performance (constant time) for these operations and thus be even faster. ArrayList would be faster if you really need to access by index but would cause the remove to go back to O(N).

ema-api#productjava
icon clock
10 |1500

Up to 2 attachments (including images) can be used with a maximum of 512.0 KiB each and 1.0 MiB total.

flame.pngflame graph from DataDog APM profiler which shows where all the CPU is used

flame.png (128.1 KiB)

1 Answer

· Write an Answer
Upvotes
Accepted
78.6k 248 52 74

@daniel.lipofsky

Thank you so much for sharing this finding.

Please raise it to the development team direclty via GitHub. Therefore, the development team can review the code.


icon clock
10 |1500

Up to 2 attachments (including images) can be used with a maximum of 512.0 KiB each and 1.0 MiB total.

I raised https://github.com/Refinitiv/Real-Time-SDK/issues/247

@Jirapongse can you please make sure the devs see it? I feel like I can't go into production with this serious of a performance issue.

@daniel.lipofsky

Yes. The development team always monitors the issues on GitHub.

Otherwise, if you have Refinitiv Developer Connect (RDC) contacts, you can also rasie this request via Contact Premium Support.

Write an Answer

Hint: Notify or tag a user in this post by typing @username.

Up to 2 attachments (including images) can be used with a maximum of 512.0 KiB each and 1.0 MiB total.