-
Notifications
You must be signed in to change notification settings - Fork 365
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
Perfomance analysis about #551 #559
Comments
Do you remember, if there are |
As far as I know there is only 2 |
Some of the memory usage seems to be caused by over-use of |
Did you plan to push a commit with less AtomicReference ? Or do you prefer thant I try to do it on my side ? |
I currently play with that. I guess I will push beginning next week. |
The gc/heap stuff seems to be improved with using a smaller stripe object and not the Exchange itself. |
See new commits in #551 |
I looked at the github version of stripedExecutor and it uses an IdentityHashMap instead of HashMap I also began to discuss about performance issue with the creator of StripedExecutorService. I will test tomorrow the changes you done in #551. |
I guess, the |
My last results for Test1: Using cf-benchmark from #551 with last modification (commit dca4c1d) and DEBUG = false (so without` new Throwable`
Using cf-benchmark from #551 with last modification (commit dca4c1d) and DEBUG = false (so without` new Throwable` and without using stripedExecutor
Interpretations: Removing |
Any further progress? |
Nope, I tried several little changes without real impact. I plan to try to reproduce a test environment near than what we have in production to see how this behaves. |
Any news? FMPOV, having a "small" performance penalty (it will be even smaller, if coaps, larger messages and/or deduplication is used), is the smaller problem than having leaks. |
I'm working on it but first tests seems to show that performance penalty is limited. |
@sbernard31 |
We can close it. I will reopen it if needed. |
Hi there, why closed it , was it fixed in new code? seems we met the same issue. The cpus are not busy, and lots of DTLS-Connection-Handler are waiting for task. Cpu: 6.7, threadId: 310827, "DTLS-Connection-Handler-2" #120 daemon prio=5 os_prio=0 tid=0x00000000016f2000 nid=0x4be2b waiting on condition [0x00007f8b4e6e7000] |
Maybe you can give more information about your setup? The benchmark results above are from plain coap, so it's no wonder that the |
@boaks is right your issue is not linked to this performances tests. You should open a new issue and give us more information to help to understand. |
Hi, boaks, thanks for the quick response. |
How many "ConnectionThread"s do you use for the DTLSConnector? |
it's default, should be 6 * 8Cpu = 48 |
So let me point out: For a more general performance discussion I updated the demos and pushed the current benchmark setup to PR #607. Using that, I start the cf-extplugtest-server/ExtendedTestServer with: -d64 -Xmx4g -XX:+UseG1GC -onlyDtlsLoopback -noPlugtest and the cf-extplugtest-client/BenchmarkClient -d64 -Xmx4g -XX:+UseG1GC coaps://localhost:5784/benchmark?rlen=200 1000 5000 My results are:
My CPU monitor shows, that the CPU is almost busy, so I don't see, that there are principal "bottlenecks" caused by "waiting threads". For an improvement in that particular scenario, it would be required to find something "waiting busy", but I hope this is not the case. So, maybe you try to adapt your benchmark setup closer to PR #607 and we will see, if there could be something found, to improve the performance. But I agree with @sbernard31, that this general issue is only related to particular issue. So if you want to provide your results, please open an other issue. |
@joechen2010 Did you try my experimental "lockless" implementation of StripedExecutor ? Personally, I didn't find any positive impact with my tests, that's why I leave this way for now :/ |
Thanks for your comments. |
Hi, we found the main performance penalty is the decrypt the message in CCMBlockCipher. the cipher init every message and it 's a synchronized action, should be move to DTLSession when it creating. |
This issue aims to regroup discussion/tests about performance analysis of #551.
Test 1
Protocol:
testing
cf-benchmark
server behavior. (as it does not changed in #551) and see how modifications impact it.using
cf-extplugtest-client
as client (I always use the same version of the client commit 4e86856, only version of server change)For each run, I launched client command 3 times in a row. (For visual VM chart I terminate performing a manual GC).
Results:
Using cf-benchmark before #551. (commit 4e86856)
Using cf-benchmark from #551 (commit 54828e7 rebased on 4e86856 to benefit of maven log fix)
Using cf-benchmark from #551 removing all `new Throwable`
Interpretations:
We note there is more CPU activities and especially more GC activities.
It seems we also allocate more heap space.
The number of request/seconds seems lower too.
Removing call to
new Throwable
sounds to improve it a bit but this seems not so smooth than before.The text was updated successfully, but these errors were encountered: