Repository navigation
Provide a way to customize the concrete Map type #3857
Copy link
Copy link
Closed
Labels
keep-openTells Stale Bot to keep PRs and issues openTells Stale Bot to keep PRs and issues open
Description
Hello,
in our projects we would like to leverage a different java.util.Mapimplementation (e.g. eclipse-collections or a custom immutable map that preserves the order). Currently we have the following class to do that:
package graphql.execution;
import graphql.ExecutionResult;
import graphql.ExecutionResultImpl;
import graphql.execution.incremental.DeferredExecutionSupport;
import graphql.execution.instrumentation.ExecuteObjectInstrumentationContext;
import graphql.execution.instrumentation.Instrumentation;
import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.function.BiConsumer;
/**
* Base class to control how the map is build. Must be in this graphql.execution package because it used
* package-private methods.
*/
public abstract class CustomizableExecutionStrategy extends AsyncExecutionStrategy {
// copy-pasted code
protected Object /* CompletableFuture<Map<String, Object>> | Map<String, Object> */
executeObject(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException {
DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = executionContext.getDataLoaderDispatcherStrategy();
dataLoaderDispatcherStrategy.executeObject(executionContext, parameters);
Instrumentation instrumentation = executionContext.getInstrumentation();
InstrumentationExecutionStrategyParameters instrumentationParameters = new InstrumentationExecutionStrategyParameters(executionContext, parameters);
ExecuteObjectInstrumentationContext resolveObjectCtx = ExecuteObjectInstrumentationContext.nonNullCtx(
instrumentation.beginExecuteObject(instrumentationParameters, executionContext.getInstrumentationState())
);
List<String> fieldNames = parameters.getFields().getKeys();
DeferredExecutionSupport deferredExecutionSupport = createDeferredExecutionSupport(executionContext, parameters);
Async.CombinedBuilder<FieldValueInfo> resolvedFieldFutures = super.getAsyncFieldValueInfo(executionContext, parameters, deferredExecutionSupport);
CompletableFuture<Map<String, Object>> overallResult = new CompletableFuture<>();
List<String> fieldsExecutedOnInitialResult = deferredExecutionSupport.getNonDeferredFieldNames(fieldNames);
BiConsumer<List<Object>, Throwable> handleResultsConsumer = buildFieldValueMap(fieldsExecutedOnInitialResult, overallResult, executionContext);
resolveObjectCtx.onDispatched();
Object fieldValueInfosResult = resolvedFieldFutures.awaitPolymorphic();
if (fieldValueInfosResult instanceof CompletableFuture) {
CompletableFuture<List<FieldValueInfo>> fieldValueInfos = (CompletableFuture<List<FieldValueInfo>>) fieldValueInfosResult;
fieldValueInfos.whenComplete((completeValueInfos, throwable) -> {
if (throwable != null) {
handleResultsConsumer.accept(null, throwable);
return;
}
Async.CombinedBuilder<Object> resultFutures = fieldValuesCombinedBuilder(completeValueInfos);
dataLoaderDispatcherStrategy.executeObjectOnFieldValuesInfo(completeValueInfos, parameters);
resolveObjectCtx.onFieldValuesInfo(completeValueInfos);
resultFutures.await().whenComplete(handleResultsConsumer);
}).exceptionally((ex) -> {
// if there are any issues with combining/handling the field results,
// complete the future at all costs and bubble up any thrown exception so
// the execution does not hang.
dataLoaderDispatcherStrategy.executeObjectOnFieldValuesException(ex, parameters);
resolveObjectCtx.onFieldValuesException();
overallResult.completeExceptionally(ex);
return null;
});
overallResult.whenComplete(resolveObjectCtx::onCompleted);
return overallResult;
} else {
List<FieldValueInfo> completeValueInfos = (List<FieldValueInfo>) fieldValueInfosResult;
Async.CombinedBuilder<Object> resultFutures = fieldValuesCombinedBuilder(completeValueInfos);
dataLoaderDispatcherStrategy.executeObjectOnFieldValuesInfo(completeValueInfos, parameters);
resolveObjectCtx.onFieldValuesInfo(completeValueInfos);
Object completedValuesObject = resultFutures.awaitPolymorphic();
if (completedValuesObject instanceof CompletableFuture) {
CompletableFuture<List<Object>> completedValues = (CompletableFuture<List<Object>>) completedValuesObject;
completedValues.whenComplete(handleResultsConsumer);
overallResult.whenComplete(resolveObjectCtx::onCompleted);
return overallResult;
} else {
Map<String, Object> fieldValueMap = buildFieldValueMap(fieldsExecutedOnInitialResult, (List<Object>) completedValuesObject);
resolveObjectCtx.onCompleted(fieldValueMap, null);
return fieldValueMap;
}
}
}
private static Async.CombinedBuilder<Object> fieldValuesCombinedBuilder(List<FieldValueInfo> completeValueInfos) {
Async.CombinedBuilder<Object> resultFutures = Async.ofExpectedSize(completeValueInfos.size());
for (FieldValueInfo completeValueInfo : completeValueInfos) {
resultFutures.addObject(completeValueInfo.getFieldValueObject());
}
return resultFutures;
}
private BiConsumer<List<Object>, Throwable> buildFieldValueMap(List<String> fieldNames, CompletableFuture<Map<String, Object>> overallResult, ExecutionContext executionContext) {
return (List<Object> results, Throwable exception) -> {
if (exception != null) {
handleValueException(overallResult, exception, executionContext);
return;
}
final Map<String, Object> resolvedValuesByField = buildFieldValueMap(fieldNames, results);
overallResult.complete(resolvedValuesByField);
};
}
@Override
protected BiConsumer<List<Object>, Throwable> handleResults(ExecutionContext executionContext, List<String> fieldNames, CompletableFuture<ExecutionResult> overallResult) {
return (List<Object> results, Throwable exception) -> {
if (exception != null) {
handleNonNullException(executionContext, overallResult, exception);
return;
}
final var map = buildFieldValueMap(fieldNames, results);
overallResult.complete(new ExecutionResultImpl(map, executionContext.getErrors()));
};
}
/**
* This is the idea of class: concrete classes can override it to provide custom implementation of the resulting Map.
*/
protected abstract Map<String, Object> buildFieldValueMap(List<String> fieldNames, List<Object> results);
}
while this is working, it is not ideal:
- it is using the package of graphql-java to access some package-private methods;
- it copy-paste a couple of methods;
- it is using inheritance: maybe delegate to a specific interface it would be better (e.g. something like ExceptionHandler, with a default implementation that produces
LinkedHashMapas by now).
I guess other integrations and projects could also benefit by this extra customization point.
I would be happy to hear what do you think about it.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
keep-openTells Stale Bot to keep PRs and issues openTells Stale Bot to keep PRs and issues open
Activity
dfa1 commentedon Mar 31, 2025
@bbakerman @andimarek hello! would you be interested in a pull request? so we could open the discussion
andimarek commentedon Apr 3, 2025
@dfa1 In general such a customization option make sense. If you create a PR, we would be happy to review/discuss further. Thanks a lot
bbakerman commentedon Apr 14, 2025
Whats the purpose of this - what does it do for you thats super important?
This seems to involve adding "extra callbacks / factories" to get this in place and hence complicates the code base
Which is ok if it has a nett benefit in say performance or memory usage
So why do you want your own Map ?
dfa1 commentedon Apr 14, 2025
Ok, a bit of context is needed then.
At SIX, we are running a lot of GraphQL-based services on kubernetes. For various reasons, we must run these services with
-xmx6gso they can scheduled more easily across any node of the cluster.By profiling and capturing the Java heap, we observed multiple times that most of the Java heap consists of
LinkedHashMap: in one case we observed 1GB of Java heap for a single response (and no, we can't paginate over it).Most of our services do something like:
So, for us, there is no point in having a map that is optimized for O(1)
get()as we just serialized it and sent it over the wire.I guess this is true for a lot of graphql-java users too, right?
Finally, we are interested in using eclipse-collections https://medium.com/oracledevs/unifiedmap-how-it-works-48af0b80cb37
in particular the
UnifiedMap:Even if the this implementation of
java.util.Mapis a bit slower when used as generic read/write map, the lower memory consumption and improved cache locality are really nice and it is a good trade off when used as GraphQL result.Merge pull request #3894 from dfa1/proposal/3857-custom-map