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

Couple tweaks to generate less garbage #451

Merged
merged 9 commits into from
Dec 20, 2017

Conversation

lutovich
Copy link
Contributor

PR makes the following changes:

  • static final CompletableFutures completed with null and false
  • improved initial sizing of HashMaps so that they do not resize
  • BoltServerAddress#equals() now checks port first and then host
  • ActiveChannelTracker only stores channel counts, not actual channels
  • improved #listAsync() to transform records buffered in the queue

Added static final instances of `CompletableFuture` completed with
`null` and `false`. This is done to reduce object allocation.
`HashMap` has default load factor of 0.75. It is often not enough to
specify desired initial capacity in order for the map to not resize.

This commit adds a helper method to create `HashMap` that will not
resize.
Because it is faster to check port integers and only then
check host strings.
`CompletionStage` that holds current connection can't be failed because
all exceptions are handled when writing to the field. This commit
removes unneeded method that tried to handle all exceptions.
It previously needed to store channels for purging of network
connections on error. Now this functionality is not needed because
driver never performs an active purge.

This commit makes `ActiveChannelTracker` only store number of active
connections per host, not the actual connections.
Summary is created when driver receives SUCCESS or FAILURE for PULL_ALL
message. It previously had it's own special handling of SUCCESS and
FAILURE in the response handler.

This commit simplifies it by making summary piggyback on FAILURE.
This test is often the first one to download and install shared neo4j
database. It might take more than 20 seconds to do this on a slow
connection. Thus timeout is increased to 60 seconds.
Method `InternalStatementResultCursor#listAsync()` returns a future
containing list of all records. Records can potentially be transformed
using the `#listAsync(Function<Record, T>)` overload. Implementation
used to be built on top of `#nextAsync()` and invoked it recursively
in `ForkJoinPool.commonPool` workers. This resulted in quite a lot of
garbage generation because every record needed to be wrapped in a
`CompletableFuture`. Also execution in the separate threads resulted
in increased CPU load.

`PullAllResponseHandler` is responsible for buffering of incoming
records in a queue. This commit makes `#listAsync()` implementation
wait for final PULL_ALL message and then transform the queue into list.
When FAILURE is received for PULL_ALL resulting future is failed. When
SUCCESS is received for PULL_ALL given transformation function is
applied to the queue of records and resulting list is returned. This
results in less garbage being generated because no intermediate
`CompletableFuture`s are created.
Copy link
Contributor

@ali-ince ali-ince left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 with one small comment.

@@ -276,7 +277,7 @@ public TypeSystem typeSystem()
{
if ( connectionStage == null )
{
return completedFuture( false );
return completedWithFalse();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since connectionStage is initialized with completedWithNull, this connectionStage == null check can be removed or checked against completedWithNull.

@ali-ince ali-ince merged commit 437dea4 into neo4j:1.5 Dec 20, 2017
@lutovich lutovich deleted the 1.5-gc-improvements branch December 20, 2017 11:47
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