Benchmark to test GEODE-7763 #127
Conversation
In this benchmark a client has many thread (180) which all try and access/modify the same key. This is to test the performance of GEODE since the removal of the client Futures cache, which was removed in 1.10.
…ReferenceBenchmark
| public void run(TestContext context) throws Exception { | ||
| PoolFactory poolFactory = PoolManager.createFactory(); | ||
| poolFactory | ||
| .addLocator("localhost", 10334) |
There was a problem hiding this comment.
See other tests on how to get the locator and port. This falsely assumes the locator is localhost but when running on multiple hosts this may not be the case.
| } | ||
| } | ||
|
|
||
| private class CreateClientPool implements Task { |
There was a problem hiding this comment.
May want to consider extending the existing CreateClient Task.
| @Override | ||
| public TestConfig configure() { | ||
| TestConfig config = GeodeBenchmark.createConfig(); | ||
| config.threads(180); |
There was a problem hiding this comment.
See the tests where these are not hard coded.
| public boolean test(Map<Object, Object> ctx) throws Exception { | ||
| int sessionAttributeCount = runSessionWorkload(); | ||
|
|
||
| assert sessionAttributeCount == this.existingSessionAttributeNames.size(); |
There was a problem hiding this comment.
Leave assert out of here. We never run with asserts enabled. Write a unit test for this benchmark that asserts the behavior you expect.
| return session; | ||
| } | ||
|
|
||
| private int runSessionWorkload() { |
There was a problem hiding this comment.
The workload is fixed and small so this seems like an overly complicated version of:
return workload1() + workload2() + workload3();
We end up benchmarking the cost of streams and lambdas too.
| private final List<String> existingSessionAttributeNames = | ||
| new ArrayList<>(); | ||
|
|
||
| private final Random random = new Random(System.currentTimeMillis()); |
There was a problem hiding this comment.
Random isn't re-entrant and can be a bottleneck if 180 threads are trying to access it. Consider ThreadLocalRandom.
| return () -> { | ||
| Session session = findById(this.sessionId.get()); | ||
|
|
||
| String name = UUID.randomUUID().toString(); |
There was a problem hiding this comment.
UUID.randomUUID() utilizes secure random to generate an array of bytes, which is not only more computationally more complex it also suffered similar contention issues that Random has. Also keeping in mind that those bytes are transient allocations and have impact on the GC and benchmark measurements.
If UUID is desired then you may want to construct UUIDs byte fetching 2 longs from ThreadLocalRandom.
This benchmark is to test the use case where many threads (on the same client) try to access/modify the same key. With the removal of the client's Future cache in 1.10, there was a performance degradation that went unnoticed and unmeasured.
This benchmark is there to make sure that this does not go unnoticed again.