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.

Sign up for GitHub

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

Jump to bottom

Performance problem with release 3.1.16 and 3.2.5 #738

Closed
torstenkuhnhenne opened this issue Jan 10, 2020 · 4 comments · Fixed by #739
Closed

Performance problem with release 3.1.16 and 3.2.5 #738

torstenkuhnhenne opened this issue Jan 10, 2020 · 4 comments · Fixed by #739
Assignees
@michael-simons
Labels
performance regression

Comments

@torstenkuhnhenne
Copy link
Contributor

After upgrading from 3.1.15 to release 3.1.16 I have massive performance problems.

I did some profiling and most of the time is spent in DomainInfo.getClassInfo():
image

I also debugged into the code and the problem seems to occur for queries that do not return a domain object but a simple type, e.g. int or String.

For these types the Map provided to DomainInfo.getClassInfo() contains no entry and so the "slow path" via regex and stream is used to find out that there is no mapping available. And then the scalar mapping is applied in ExecuteQueriesDelegate.executeAndMap(Class<T>, String, Map<String, ?>)

I also tried the release 3.2.5 and this has also the performance problem.

I added an additional cache to the method, so that the result of "slow path" is cached and that resolved the performance problem for me, see torstenkuhnhenne@f55372f

Expected Behavior

The new releases 3.1.16 and 3.2.5 should be as fast as 3.1.15.

Current Behavior

The releases 3.1.16 and 3.2.5 are much slower than 3.1.15.

Possible Solution

Downgrade to 3.1.15

Your Environment

  • OGM Version used: 3.1.16 / 3.2.5
  • Java Version used: 11
  • Neo4J Version used: 3.5.14
  • Bolt Driver Version used (if applicable): 1.7.5 / 4.0.0
  • Operating System and Version: Windows 10
@michael-simons
Copy link
Collaborator

michael-simons commented Jan 10, 2020 •
edited
Loading

Thanks for your detailed report. We're gonna investigate this.

Related to the fix for #678.

I understand the problem and I like the idea of your fix and I think I'm gonna use most of it.

Would you be able to test a feature branch in your system? We can fix this in a patch release.

@michael-simons michael-simons self-assigned this Jan 10, 2020
michael-simons added a commit that referenced this issue Jan 10, 2020
@michael-simons
GH-738 - Improve performance of fully qualified class name lookup.
0ae5510 
GH-678 and the changes related to GH-391 introduced a performance issue
with regards of looking up fully qualified class names.

This change addresse it by:

* Not using the stream api (again)
* Caching the result of the lookup

As the later should happens thread safe, we use a `ConcurrentHashMap`,
so we cannot store null values, but optional references.
@michael-simons michael-simons mentioned this issue Jan 10, 2020
Merged
michael-simons added a commit that referenced this issue Jan 10, 2020
@michael-simons
GH-738 - Improve performance of fully qualified class name lookup.
9406b41 
GH-678 and the changes related to GH-391 introduced a performance issue
with regards of looking up fully qualified class names.

This change addresse it by:

* Not using the stream api (again)
* Caching the result of the lookup

As the later should happens thread safe, we use a `ConcurrentHashMap`,
so we cannot store null values, but optional references.
@michael-simons
Copy link
Collaborator

michael-simons commented Jan 10, 2020 •
edited
Loading

Hey @torstenkuhnhenne could you please have a look if either #740 (Patch for 3.1) or #739 (Patch for Master / 3.2) solves this for you?

@torstenkuhnhenne
Copy link
Contributor Author

Hi @michael-simons I have tested both patches and can confirm that they solve the problem! :-)

Thank you for the quick response and the good work :-)

@michael-simons
Copy link
Collaborator

🙇 thank you. Expect a patch next week. I’ll see if I can address the other new issue on Monday.

michael-simons added a commit that referenced this issue Jan 13, 2020
@michael-simons
GH-738 - Improve performance of fully qualified class name lookup.
0934b12 
GH-678 and the changes related to GH-391 introduced a performance issue
with regards of looking up fully qualified class names.

This change addresse it by:

* Not using the stream api (again)
* Caching the result of the lookup

As the later should happens thread safe, we use a `ConcurrentHashMap`,
so we cannot store null values, but optional references.
michael-simons added a commit that referenced this issue Jan 13, 2020
@michael-simons
GH-738 - Improve performance of fully qualified class name lookup.
2df1b3d 
GH-678 and the changes related to GH-391 introduced a performance issue
with regards of looking up fully qualified class names.

This change addresse it by:

* Not using the stream api (again)
* Caching the result of the lookup

As the later should happens thread safe, we use a `ConcurrentHashMap`,
so we cannot store null values, but optional references.
michael-simons added a commit that referenced this issue Jan 13, 2020
@michael-simons
GH-738 - Improve performance of fully qualified class name lookup.
2185e11 
GH-678 and the changes related to GH-391 introduced a performance issue
with regards of looking up fully qualified class names.

This change addresse it by:

* Not using the stream api (again)
* Caching the result of the lookup

As the later should happens thread safe, we use a `ConcurrentHashMap`,
so we cannot store null values, but optional references.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michael-simons michael-simons

Labels
performance regression
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

GH-738 - Improve performance of fully qualified class name lookup. neo4j/neo4j-ogm
2 participants
@michael-simons @torstenkuhnhenne