graphql-java / graphql-java/graphql-java

Look into memory allocation of ExecutionStrategyParameters

未关闭
#3,933 1 条评论 1 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

keep-open
主要语言
Java
星标
6.2k
派生
1.1k
平均合并
22 分钟
30 天内合并 PR
14

描述

Many many moons ago the graphql.execution.ExecutionStrategy methods use to take a wide and variety of parameters.

In the name of more maintainable code, the ExecutionStrategyParameters object was created to provide a standard set of parameters to to each method.

So you will see it change as the engine executes each field like this say

            ExecutionStrategyParameters newParameters = parameters
                    .transform(builder -> builder.field(currentField).path(fieldPath));

The thing here is we are allocating a new object each time we go into a method. Some cases might deserve it but others cases questionable AND it allocates more objects and this costs time and memory. At low scale this doesn't matter. At high scale this matters more.

For example the completeValueForList method iterates a list of objects (or scalars) and allocates a new object for each so that the "field index" changes just in case there is an error we have to handle.

Also we keep a hierarchy of these parameters (albeit not consistently) so this might also be removed in the name of performance

    public ExecutionStrategyParameters getParent() {
        return parent;
    }

We also use the builder patten so parameters.transform(builder -> builder.field(currentField).path(fieldPath)); is in fact two objects allocated and copied. The builder and the final new object.

This issue tries to document the places we change ExecutionStrategyParameters to try to outline the scope and approach we might take to avoid allocations.

The places

graphql.execution.Execution

The very start - the actual only places that use the builder outside of a transform

        ExecutionStrategyParameters parameters = newParameters()
                .executionStepInfo(executionStepInfo)
                .source(root)
                .localContext(executionContext.getLocalContext())
                .fields(fields)
                .nonNullFieldValidator(nonNullableFieldValidator)
                .path(path)
                .build();
graphql.execution.AsyncSerialExecutionStrategy

Top level fields executed in sequential order - as we resolve each field (fetch and complete) we create a new one per field.

NOTE: there can only be a final set of top level fields per request and mutations are used less then queries. So there is limited improvement possible here.

NOTE : It does not set the parent ExecutionStrategyParameters which shows that this concept of parent ExecutionStrategyParameters is of limited use.

        CompletableFuture<List<Object>> resultsFuture = Async.eachSequentially(fieldNames, (fieldName, prevResults) -> {
            MergedField currentField = fields.getSubField(fieldName);
            ResultPath fieldPath = parameters.getPath().segment(mkNameForPath(currentField));
            ExecutionStrategyParameters newParameters = parameters
                    .transform(builder -> builder.field(currentField).path(fieldPath));
graphql.execution.ExecutionStrategy#getAsyncFieldValueInfo

This is called for the top level fields of a query and for inner every object fields after that. So this provides much more potential for improvement.

Its make a new path and and MergedField object for each field present. Its also setting the parent ExecutionStrategyParameters which is not done in all places. See above

        MergedSelectionSet fields = parameters.getFields();
        ....
        for (String fieldName : fields.getKeys()) {
            MergedField currentField = fields.getSubField(fieldName);

            ResultPath fieldPath = parameters.getPath().segment(mkNameForPath(currentField));
            ExecutionStrategyParameters newParameters = parameters
                    .transform(builder -> builder.field(currentField).path(fieldPath).parent(parameters));
graphql.execution.ExecutionStrategy#completeField(graphql.schema.GraphQLFieldDefinition, graphql.execution.ExecutionContext, graphql.execution.ExecutionStrategyParameters, graphql.execution.FetchedValue)

This is called when we have fetched a source object and we need to complete the field

There is another PR for how nonNullableFieldValidator is used so this should go away

The local context needs to be capture some where to allow it to be accessed as we descend the field tree

The source object is also important

        ExecutionStrategyParameters newParameters = parameters.transform(builder ->
                builder.executionStepInfo(executionStepInfo)
                        .source(fetchedValue.getFetchedValue())
                        .localContext(fetchedValue.getLocalContext())
                        .nonNullFieldValidator(nonNullableFieldValidator)
        );

And finally executionStepInfo is where the most changes are happening here.

Really this object is the important one since it contains lots of info about what is happening

Again this is a builder pattern to 2 objects for one final object

        return newExecutionStepInfo()
                .type(fieldType)
                .fieldDefinition(fieldDefinition)
                .fieldContainer(fieldContainer)
                .field(field)
                .path(parameters.getPath())
                .parentInfo(parentStepInfo)
                .arguments(argumentValues)
                .build();
graphql.execution.ExecutionStrategy#completeValueForList(graphql.execution.ExecutionContext, graphql.execution.ExecutionStrategyParameters, java.lang.Iterable<java.lang.Object>)

This is called in a loop of the field for each list item.

It creates a new set of parameters for each item

            ExecutionStrategyParameters newParameters = parameters.transform(builder ->
                    builder.executionStepInfo(stepInfoForListElement)
                            .nonNullFieldValidator(nonNullableFieldValidator)
                            .localContext(value.getLocalContext())
                            .path(indexedPath)
                            .source(value.getFetchedValue())

The executionStepInfo is basically the same thing with list type and field type unwrapped and the index of the list item in it

It then calls back to completeValue which may in turn create more new parameters - nulls and scalars and enums dont but objects and inner lists do.

graphql.execution.ExecutionStrategy#completeValueForObject

When this method is invoked its ready to restart the field sub selection process again.

It uses graphql.execution.FieldCollector#collectFields(graphql.execution.FieldCollectorParameters, graphql.execution.MergedField, boolean) to work out what sub fields to execute

Its also had a "object resolution" done to it. hence we have new types involved now for this object.

        ExecutionStrategyParameters newParameters = parameters.transform(builder ->
                builder.executionStepInfo(newExecutionStepInfo)
                        .fields(subFields)
                        .nonNullFieldValidator(nonNullableFieldValidator)
                        .source(result)
        );

It then calls back to graphql.execution.ExecutionStrategy#executeObject

This then comes back around to call graphql.execution.ExecutionStrategy#getAsyncFieldValueInfo again per sub field and makes new ExecutionStrategyParameters parameters again

Thoughts

The NonNullableFieldValidator PR I have up will work around that - so it can be ignored

The parent idea should be scrapped -we dont do it properly and we never read it in code. Lets kill that

The values that are changing as descend are

  • .field(currentField) - MergedField
  • .path(fieldPath) - ResultPath - including the index when we complete via a list value
  • .fields(mergedSelectionSet) - MergedSelectionSet
  • .executionStepInfo(newExecutionStepInfo) - ExecutionStepInfo
  • .source(fetchedValue.getFetchedValue()) - Object
  • .localContext(fetchedValue.getLocalContext()) - Object

A lot of these values like ResultPath are only used when a error happens

        TypeMismatchError error = new TypeMismatchError(parameters.getPath(), parameters.getExecutionStepInfo().getUnwrappedNonNullType());
Pass things around via method arguments

Are there too many changing parameters to just pass them around rather than wrapping them up in the ExecutionStrategyParameters holder?? I dont think so. Some like ResultPath might be tricky but "source" can be passed around easily enough as can ExecutionStepInfo etc..

imagine

someMethod(ExecutionContext ec, ExecutionStrategyParameters es, ExecutionStepInfo esi, ResultPath path, int index, Object source, Object localContext, MergedField field, MergedSelectionSet selectionSet)

Its not as neat but it is efficient in that you are not building 2 objects (ExecutionStrategyParameters + its Builder) to set 3 values into it at a time.

This is a regression back to the old days :)

This is what it might have looked like in 2015


public abstract ExecutionResult execute(ExecutionContext executionContext, GraphQLObjectType parentType, Object source, Map<String, List<Field>> fields);



protected ExecutionResult resolveField(ExecutionContext executionContext, GraphQLObjectType parentType, Object source, List<Field> fields)
``

the `index` might be `-1` for non lists and `0..N` and then lazy assembled should there be an error say



### Instrumentation


`ExecutionStrategyParameters` is a key parameter in the `Instrumentation` area .  This is really hard to ensure it has the same API shape if we start to change it

We could be using "lazy suppliers" to build ExecutionStrategyParameters if we need to get them to Instrumentation in the same semantic way.





贡献指南

打开贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

调研方向

从 graphql.execution.ExecutionStrategyParameters 开始,跟踪它通过 Execution、AsyncSerialExecutionStrategy、ExecutionStrategy#getAsyncFieldValueInfo、completeField、completeValueForList 和 completeValueForObject 的构造过程。审查相关的 ExecutionStepInfo 和 Instrumentation 使用情况。完成的标准是记录已确定的减少分配方法及其范围,同时保留所需的参数语义。

由索引模型根据 Issue 内容生成。

评估

技术栈
java
领域
backend, performance
Issue 类型
重构
难度
5/5
预计耗时
一周以上
活跃度
停滞
描述清晰度
需要澄清
新手友好度
35/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。