GeezerVison

I think this PR is a good work around. However cheaper accessors is the way forward really

Can also discuss if there should be cheaper getters on TypeRegistry. I welcome your thoughts!

yeah the code today is old and crusty

    public Map<String, List<ObjectTypeExtensionDefinition>> objectTypeExtensions() {
        return new LinkedHashMap<>(objectTypeExtensions);
    }

This idea of copying the objects should be replaced by ImmutableXX collections - we have embraced this elsewhere in the graphql code base but not here is seems.

if we had immutables in place then

return buildCtx.getTypeRegistry().objectTypeExtensions().getOrDefault(typeDefinition.getName(), emptyList());

and

        return buildCtx.objectTypeExtensions.getOrDefault(typeDefinition.getName(), emptyList());

would be the same cost

Ahh I just had a look at the code - its embraced mutability up the wazoo

typeRegistry.add(x)

typeRegistry.merge(otherTypeReg)

Hmmm immutability is now tricky because its relying on mutability when being built (versus being read)

I am wary that we could make this embrace immutability and not break everyone - is the juice worth the squeeze ??

We need some new structure when its passed in as a "read only" world to say SchemaGenerator and so on

Spitballing this could be

ImmutableTypeRegistry locked =  typeRegistry.readOnly()

and then key code could be hanged to used here. In some ways this is what you did when you threaded the "copied" lists into the read only places

Hmmmm tricky to fix systemically

ok this PR is probably more systemic

#4021

Now the code covered in this PR gets a graphql.schema.idl.ImmutableTypeDefinitionRegistry and hence asking for the values repeatedly give out the same object and not a copy of the maps

That's great @bbakerman 🚀🚢

Closing in favor of #4021