Refine runtime hint conditions

[sdeleuze]

GraalVM conditional configuration is only documented for reflection but it also applies to serialization, proxy and resources configurations. Since this mechanism allows to leverage GraalVM static analysis to reduce the memory footprint and image size, and for consistency, it would make sense to not only expose this capability in ReflectionHints, but also in JavaSerializationHints, ProxyHints and ResourceHints.

[sdeleuze]

After various discussion with GraalVM team and @bclozel, I think we need to make the reachableType mandatory in our API since:

• It allows to keep track of the reason of the inclusion of a hint
• It improves the footprint by leveraging GraalVM static analysis, not just basic classpath checks
• This is consistent with the design decision on GraalVM native configuation side which as been renamed to jvm-reachability-metadata to emphasis the reachability is a central concern here
• It will likely allow us to have the same maintenability issue that on Spring Native with @NativeHint

Since annotation are currently not supported yet as relevant trigger (see related GraalVM issue), we should implement a check to verify annotations are not used as reachableType.

[snicoll]

we should implement a check to verify annotations are not used as reachableType.

You can't reliably do that I am afraid. TypeReference won't tell you that. We do have specific checks for methods that take a Class as a shortcut though.

[sdeleuze]

You can't reliably do that I am afraid. TypeReference won't tell you that. We do have specific checks for methods that take a Class as a shortcut though.

That's not a hard requirement, we can at least document it and do the check when we have the class. We could also maybe perform this check via ASM-based utility methods (I think we already have that implemented) when generating the config files since this will happen AOT.

[sdeleuze]

@snicoll About your remark on type = reachableType in Spring Native, that's the case in a lot of @NativeHint indeed because we have a lot of hints that are using a "Spring trigger" which has a special meaning and special processing (it does not translate to reachableType in this case).

What I would like to do in Framework 6 is removing this special processing for "Spring things" which is not clearly defined and can't be check in a reliable way, and always translate to regular reachableType.

Since that's sadly not properly documented yet on GraalVM side, see examples of such files from the jvm-reachability-metadata repository:
proxy-config.json

[
  {
    "condition": {
      "typeReachable": "org.jline.utils.Signals"
    },
    "interfaces": [
      "sun.misc.SignalHandler"
    ]
  }
]

resource-config.json

{
  "bundles": [],
  "resources": {
    "includes": [
      {
        "condition": {
          "typeReachable": "org.jline.terminal.TerminalBuilder"
        },
        "pattern": "\\QMETA-INF/services/org.jline.terminal.spi.JansiSupport\\E"
      }
  ]
}

reflect-config.json

[
  {
    "condition": {
      "typeReachable": "org.jline.terminal.impl.jansi.JansiNativePty"
    },
    "name": "java.io.FileDescriptor",
    "queriedMethods": [
      {
        "name": "<init>",
        "parameterTypes": [
          "int"
        ]
      }
    ]
  }
]

[philwebb]

We might be able to use com.oracle.svm.core.configure.ResourceConfigurationParser in the JUnit test to verify the JSON without needing a native image.

[sdeleuze]

@snicoll Could you please have a look to this draft branch where:

• I made reachable type mandatory by introducing condition since more could come later (see related commit)
• I added reachable type to ResourceHints as well (see related commit)

If you test it with sample native application, notice GraalVM 22.1-dev is required since GraalVM 22.0 is broken for some kind of conditional configuration.

[snicoll]

I had a look to the two commits and added some comments.

I don't think making the reachable type mandatory in the API is warranted when the commit does this at the same time. If we can't figure out a better reachable type in AutowiredAnnotationBeanPostProcessor, then I don't think we should be forced to write something like this.

I also think that making it mandatory in the method that creates the builder makes the API more fat. I'd welcome a different arrangement. Perhaps a validation that a condition is set when building the object? Still, I think it makes the API harder to use than it should and I am wondering if this is warranted.

[bclozel]

If I understand correctly, the GraalVM agent will use as reachableType whatever type is right above in the call stack when performing a reflection/resource/etc call. That's a nice heuristic for many libraries, but I suspect that this approach won't be super efficient in Spring Framework.

In many cases, the call stack with start with a ResourceUtils, ClassUtils, etc. class that does not really help discarding useless hints. I think that two cases might happen in our case:

• we will look higher in the call stack and select a type that's more representative of the use case, for example ResourceWebHandler for serving static resources
• we will point instead to a configuration class that will not be in the call stack but still represents well the trigger for many features. For example, WebMvcConfigurationSupport.

I don't think making the reachable type mandatory in the API is warranted when the commit does this at the same time. If we can't figure out a better reachable type in AutowiredAnnotationBeanPostProcessor, then I don't think we should be forced to write something like this.

I agree with the comment above. On top of that, I believe some hints might not even have a valid reachableType. In the case of field injection, we would have to point to a class that's always reachable by the core context - or worse, point to an AOT-generated class. In both cases, this doesn't really help.

To summarize, I'd like to add this option to the API but not make it mandatory.

[sdeleuze]

Ok thanks for your feedback, I think I will just make a new try with reachable types added to other hints than reflection one and let it optional.

[bclozel]

To summarize our findings so far:

• Hints conditions are important and we want to support them. So far typeReachable is the only existing one, and its main use case is about reflection metadata. But we've seen conditions used in other places like proxy-config or resource-config.
• typeReachable and hints conditions should not be mandatory in our API
• we can't necessarily automate the condition generation; typeReachable is about a particular type being reachable by the static analysis and doesn't necessarily means that it is part of the call stack

With this change, we can consider #28163 with a more general approach.