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

Minor performance fixes #3236

Merged
merged 4 commits into from Jun 2, 2023
Merged

Conversation

dfa1
Copy link
Contributor

@dfa1 dfa1 commented May 30, 2023

hello @bbakerman @andimarek :)

another batch of small fixes... please have a look when you have time :)

D.

GraphQLList fieldType = (GraphQLList) executionInfo.getUnwrappedNonNullType();
GraphQLOutputType typeInList = (GraphQLOutputType) fieldType.getWrappedType();
ResultPath indexedPath = executionInfo.getPath().segment(index);
Copy link
Contributor Author

@dfa1 dfa1 May 30, 2023

Choose a reason for hiding this comment

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

according to my memory profiler, this is dropping from 3-4% of total heap to just 1-2% (as expected)

Copy link
Member

Choose a reason for hiding this comment

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

Just to explain to others - the indexedPath was allocated out side this method and then reallocated again here. This double allocation has been removed

@dfa1 dfa1 changed the title Reuse result path Minor performance fixes Jun 1, 2023
@dfa1
Copy link
Contributor Author

dfa1 commented Jun 1, 2023

master branch 110afe0

# Run progress: 0.00% complete, ETA 00:06:40
# Fork: 1 of 5
# Warmup Iteration   1: 59.620 ops/s
# Warmup Iteration   2: 81.651 ops/s
Iteration   1: 62.665 ops/s
Iteration   2: 63.098 ops/s
Iteration   3: 63.295 ops/s

# Run progress: 10.00% complete, ETA 00:06:11
# Fork: 2 of 5
# Warmup Iteration   1: 60.147 ops/s
# Warmup Iteration   2: 82.813 ops/s
Iteration   1: 63.977 ops/s
Iteration   2: 64.611 ops/s
Iteration   3: 64.701 ops/s

# Run progress: 20.00% complete, ETA 00:05:29
# Fork: 3 of 5
# Warmup Iteration   1: 62.425 ops/s
# Warmup Iteration   2: 86.494 ops/s
Iteration   1: 66.016 ops/s
Iteration   2: 65.639 ops/s
Iteration   3: 67.224 ops/s

# Run progress: 30.00% complete, ETA 00:04:48
# Fork: 4 of 5
# Warmup Iteration   1: 60.987 ops/s
# Warmup Iteration   2: 82.945 ops/s
Iteration   1: 63.604 ops/s
Iteration   2: 64.055 ops/s
Iteration   3: 64.656 ops/s

# Run progress: 40.00% complete, ETA 00:04:07
# Fork: 5 of 5
# Warmup Iteration   1: 58.898 ops/s
# Warmup Iteration   2: 80.509 ops/s
Iteration   1: 62.096 ops/s
Iteration   2: 62.666 ops/s
Iteration   3: 62.697 ops/s


Result "benchmark.BenchMark.benchMarkSimpleQueriesThroughput":
  64.067 ±(99.9%) 1.531 ops/s [Average]
  (min, avg, max) = (62.096, 64.067, 67.224), stdev = 1.432
  CI (99.9%): [62.535, 65.598] (assumes normal distribution)

this branch:

# Run progress: 0.00% complete, ETA 00:06:40
# Fork: 1 of 5
# Warmup Iteration   1: 61.105 ops/s
# Warmup Iteration   2: 86.944 ops/s
Iteration   1: 66.475 ops/s
Iteration   2: 67.328 ops/s
Iteration   3: 67.736 ops/s

# Run progress: 10.00% complete, ETA 00:06:11
# Fork: 2 of 5
# Warmup Iteration   1: 67.949 ops/s
# Warmup Iteration   2: 98.241 ops/s
Iteration   1: 75.801 ops/s
Iteration   2: 76.467 ops/s
Iteration   3: 76.391 ops/s

# Run progress: 20.00% complete, ETA 00:05:29
# Fork: 3 of 5
# Warmup Iteration   1: 66.094 ops/s
# Warmup Iteration   2: 86.489 ops/s
Iteration   1: 66.301 ops/s
Iteration   2: 66.677 ops/s
Iteration   3: 67.043 ops/s

# Run progress: 30.00% complete, ETA 00:04:48
# Fork: 4 of 5
# Warmup Iteration   1: 67.452 ops/s
# Warmup Iteration   2: 92.578 ops/s
Iteration   1: 71.087 ops/s
Iteration   2: 71.555 ops/s
Iteration   3: 71.605 ops/s

# Run progress: 40.00% complete, ETA 00:04:07
# Fork: 5 of 5
# Warmup Iteration   1: 63.314 ops/s
# Warmup Iteration   2: 89.314 ops/s
Iteration   1: 68.632 ops/s
Iteration   2: 69.317 ops/s
Iteration   3: 69.883 ops/s


Result "benchmark.BenchMark.benchMarkSimpleQueriesThroughput":
  70.153 ±(99.9%) 3.861 ops/s [Average]
  (min, avg, max) = (66.301, 70.153, 76.467), stdev = 3.612
  CI (99.9%): [66.292, 74.015] (assumes normal distribution)


@Internal
public class ExecutionStepInfoFactory {

public ExecutionStepInfo newExecutionStepInfoForSubField(ExecutionContext executionContext, MergedField mergedField, ExecutionStepInfo parentInfo) {
Copy link
Member

Choose a reason for hiding this comment

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

just checked - code was never used

public int getCurrentListIndex() {
return currentListIndex;
}

Copy link
Member

Choose a reason for hiding this comment

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

There are not used anywhere and are informational

@bbakerman
Copy link
Member

Thanks

@bbakerman bbakerman added this pull request to the merge queue Jun 2, 2023
@bbakerman bbakerman added this to the 2023 July milestone Jun 2, 2023
@bbakerman bbakerman added the performance work that is primarily targeted as performance improvements label Jun 2, 2023
Merged via the queue into graphql-java:master with commit 7905df0 Jun 2, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance work that is primarily targeted as performance improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants