Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TransactionServiceImp optimisations #200

Conversation

nikhildp
Copy link

We are using atomikos in our product. Below are some optimisations I noticed based on observations from production systems.
I have converted tidToTransactionMap_ to concurrent hashmap as it is more efficient in handling concurrent operations. Also, thread safe objects doesn't need locks for operations. Hence removed the same for tidToTransactionMap_ put/read operations.

@nikhildp
Copy link
Author

fixes: #198

@nikhildp
Copy link
Author

To elaborate on the issue:
Problem is during high concurrency, threads keep waiting to register, as 1) There is a synchronised lock on HashMap variable tidToTransactionMap_ 2) Entire HashMap gets locked when there is a write operation. I have seen this in production application when we analyzed the threaddump incrementaly. With concurrent hashmap and without synchronised block. this waiting should go away. We are in progress with the benchmarking of this changes (with my local patch). I will have some numbers within in a week or 2 to confirm/disprove the theory

@nikhildp
Copy link
Author

nikhildp commented Jan 13, 2024

Here is my comparison of concurrent hashmap and synchronising on Map perf benchmarking. Doesn't come as a surprise, but Concurrent hashmap gives better performance across different configurations. Additionally latter has performance degradation with increased concurrency as there are many threads fighting for the lock simultaneously
Source code here -> https://github.com/nikhildp/MapBenchMark/tree/main
image

We are still in process of end to end perf benchmarking

@nikhildp
Copy link
Author

nikhildp commented Jan 18, 2024

image

So we have profiled this specific function in production and even with synchronisation, execution time is 0 ms on average, however, there are some threads which waited for around 200 - 500 ms consistently 9which is evident from max value). so probably for 95+ percentile, this won't be a good experience. Though minor, this change should give some improvement
set_tx_to_tid.csv

…cking all threads from creating/removing co-ordinators
… as concurrent hashmaps to avoid using random latches and improve concurrency
@GuyPardon
Copy link
Contributor

Thanks! We are looking into how to merge this in, so in preparation of that I have prepared the following documentation:

https://www.atomikos.com/Documentation/ConcurrencyModel

Any feedback welcome :-)

@nikhildp
Copy link
Author

Thanks! We are looking into how to merge this in, so in preparation of that I have prepared the following documentation:

https://www.atomikos.com/Documentation/ConcurrencyModel

Any feedback welcome :-)

@GuyPardon : this looks good to me!

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