Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement ByteArrayClassLoader::getResourceAsStream #170

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

naude-r
Copy link

@naude-r naude-r commented Jun 14, 2022

This PR adds getResourceAsStream to ByteArrayClassLoader. In our use case we need to load / access the generated byte code to add to a jar. This can optionally take place at a much later stage, ie long after compilation.

@naude-r
Copy link
Author

naude-r commented Jun 15, 2022

@aunkrig is this PR acceptable?

@aunkrig
Copy link
Member

aunkrig commented Aug 17, 2022

I adopted your PR, but slightly changed the implementation: Resources are kept in a separate map, because resource names are structured differently than class names, and should thus not be mixed.

Does this work for you?

@naude-r
Copy link
Author

naude-r commented Aug 17, 2022

I adopted your PR, but slightly changed the implementation: Resources are kept in a separate map, because resource names are structured differently than class names, and should thus not be mixed.

Does this work for you?

@aunkrig Thank you. Unfortunately this does not. We are looking for the actual class data (compiled class) and not resources per se.

for a jdk based classloader we can currently do:

var clazz = List.class;
var is = clazz.getClassLoader().getResourceAsStream(clazz.getName()));
writeToJar(is)

this allow us write the class to a new jar. this is used by a system to generate jar + classes on the fly.

hope this make sense?

@naude-r
Copy link
Author

naude-r commented Aug 29, 2022

@aunkrig Any feedback?

@aunkrig
Copy link
Member

aunkrig commented Aug 30, 2022

I understand. So why don't you just ask the Map that you passed to the ByteArrayClassLoader? Class names and resource names are not necessarily identical (apart from separator "." and suffix ".class"): Components may be truncated, or names may be case-insensitive. In other words: It is unsafe to ask a ClassLoader:

classLoader.getResourceAsStream("my/pkg/MyClass.class")

I recommend that you use the Map that you passed to the ByteArrayClassLoader directly, rather than through the class loader. Is that feasible?

@naude-r
Copy link
Author

naude-r commented Aug 30, 2022

@aunkrig We use ISimpleCompiler and cache the compiled class. The jar is optionally build only much later (minutes or even hours). We will need to store the map from ICookable::getBytecodes to get access to it. Using the ClassLoader instance is more elegant as we are also using a similar mechanism for jdk based classes, which simplifies the jar building as we can query the class for the ClassLoader instance. ByteArrayClassLoader only provides getResourceAsStream as a way to get to the class data.

If there is no other way we will have to deal with it on our end.

@naude-r
Copy link
Author

naude-r commented Aug 30, 2022

bytebuddy seems to use the same mechanism, ie using getResourceAsStream:
https://github.com/raphw/byte-buddy/blob/master/byte-buddy-dep/src/main/java/net/bytebuddy/dynamic/ClassFileLocator.java#L367

@aunkrig
Copy link
Member

aunkrig commented Aug 30, 2022

Honestly, this very tricky use of a ClassLoader is not really in the scope of Janino. I propose that you create your own NaudesClassLoader and use that!? Feel free to use ByteArrayClassLoader as a template.

@naude-r
Copy link
Author

naude-r commented Aug 31, 2022

@aunkrig Thank you for your patience, that it totally understandable.

@enexusde
Copy link

enexusde commented May 31, 2024

Why not overwrite getResource(String) with a valid implementation? Since ByteArrayClassLoader.getResource will be used by ClassLoader.getResourceAsStream correctly.

Original ClassLoader.getResourceAsStream method is:

    public InputStream getResourceAsStream(String name) {
        Objects.requireNonNull(name);
        URL url = getResource(name);
        try {
            return url != null ? url.openStream() : null;
        } catch (IOException e) {
            return null;
        }
    }

@Geolykt
Copy link
Contributor

Geolykt commented May 31, 2024

Let's not kid ourselves: The frequency in which getResource will be called directly is reduced compared to getResourceAsStream. The main disadvantage implementing getResource has over getResourceAsStream is that you'd have a lot more work regarding creating your own URL instance with the right URL protocol handler and all overhead associated with it. It's not impossible - but certainly not as easy as other methods (and DEFINITELY out of scope for janino). I would thus only do so if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants