Fix #629: Adding subscriptions is very slow

[blankbro]

refactor CTrie

[andsel]

Hi @amzexin
the PR changes a lot the original data structure which was inspired on lock free algorithms on CTrie hash.
I would prefer a PR that only resolve the problem on the loop used in the insertion. The copy of constructor, and the copy in general shouldn't be a problem, as demonstrated by the CTrie hashmap. The copy and the presence of INode (indirect node) is fundamental for correct concurrent access to data structure.

[andsel]

Hi @amzexin
thank's for your contribution but, as I described in my previous comment, the changes you are proposing other then resolving the slow search loop also completely changes the spirit of the original data structure that was inspired by CTrie.
I'm happy to review a PR that just resolve the problem on the slow search loop replacing it hash lookup.

[blankbro]

Hi @andsel
I tried to optimize CNode.copy, but it seems impossible. Because this itself may be a false proposition.

CAS is a good idea of concurrency lock-free, but it doesn't seem appropriate on CTrie. Because the original Set and List must be copied before adding a subscription or inode in CNode. Take an exaggerated example：if CNode has 1000000 Subscriptions, when we need to add the 1000001st subscription, we must copy these 1000000 Subscription first, then add 1 Subscription, then replace the original CNode by CAS, and when CTrie.insert returns Action.REPEAT, we need to do it again. And this will cause the project to start very slowly, because CTrieSubscriptionDirectory.init will call ctrie.addToTree method. CTrie added 620k Subscriptions need cost 85 minutes, this means that it takes 85 minutes for the project to start, which is very dreadful😱

InsertSubscription bottleneck reproduce: testCNodeCopyForInsertSubscription
AddChild bottleneck reproduce: testCNodeCopyForAddChild

Finally, I have a question. Ctrie must be lock-free? If the answer is yes, can you provide some optimization ideas for CNode.copy? I'm willing to try.

[andsel]

Hi @amzexin
about your questions:

Ctrie must be lock-free?

the answer is yes, because CTrie is accessed concurrently by the subscriber and publisher threads.

can you provide some optimization ideas for CNode.copy?

As was initially explained in this comment the slowness was related to the linear scan on insertion and not to the copy.
Do you have any measure if it's the copy to slow down the entire process?
As an idea to avoid the copy, we could use for children list a persistent data structure, like the one used in by functional languages, where any modification of the data structure create a new view of the data which contains only the changes. In that case we could avoid the need to copy the children array but just use the reference. Everytime we add or remove an element a new view is generated.

[hylkevds]

The immediate problem is the looping. This is especially bad because it also happens when matching subscriptions for a publish action. I think that needs to be fixed first. Hashtables are a possible solution, but simply sorting the array by token already improves things drastically. See my experiment in #630.

The copy is bad under high load, on large trees, since many failed inserts will happen, and many re-tries, and this will increase the load even further. A simple test should be made if adding a lock-on-update to the current code is faster under load than the current retry-until-success. Reading can remain lock-free that way.

[andsel]

This is already fixed by #630, please @timeway could you check if it solves your issue and in case close the PR please 🙏 ?

[blankbro]

This is already fixed by #630, please @timeway could you check if it solves your issue and in case close the PR please 🙏 ?

I use the latest code of the main branch and test it with the previous case, which is still very slow. This is my test case.

[hylkevds]

Indeed, this is yet another use-case. This one has few topics, but each topic has many subscribers.

[hylkevds]

In #758 I converted the HashSet to an ArrayList, since copying an array is much faster than copying a Set.

The test is still the slowest of the three (23 seconds), but it doesn't time out any more.