Skip to content

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.

public ExecutionStepInfo newExecutionStepInfoForListElement(ExecutionStepInfo executionInfo, ResultPath indexedPath) {
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)

@dfa1 dfa1 force-pushed the reuse-result-path branch from b919cdb to 30db887 Compare June 1, 2023 20:17
@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
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.

2 participants