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

Add reflection cache #26

Open
markzhai opened this issue Mar 3, 2016 · 5 comments
Open

Add reflection cache #26

markzhai opened this issue Mar 3, 2016 · 5 comments

Comments

@markzhai
Copy link

markzhai commented Mar 3, 2016

It would be slow if calling the same reflection methods from time to time, can the library provide this function?

Like https://github.com/Qihoo360/DroidPlugin/tree/master/project/Libraries/DroidPlugin/src/com/morgoo/droidplugin/reflect
FieldUtils.java:

public class FieldUtils {

    private static Map<String, Field> sFieldCache = new HashMap<String, Field>();
    ...
    private static Field getField(Class<?> cls, String fieldName, final boolean forceAccess) {
        Validate.isTrue(cls != null, "The class must not be null");
        Validate.isTrue(!TextUtils.isEmpty(fieldName), "The field name must not be blank/empty");

        String key = getKey(cls, fieldName);
        Field cachedField;
        synchronized (sFieldCache) {
            cachedField = sFieldCache.get(key);
        }
        if (cachedField != null) {
            if (forceAccess && !cachedField.isAccessible()) {
                cachedField.setAccessible(true);
            }
            return cachedField;
        }
        ...
    }
    ...
}
@lukaseder
Copy link
Member

Have you benchmarked this implementation via JMH? I wouldn't be surprised if the synchronisation + hashmap access is slower than the current implementation!

@rdifrango
Copy link

I'm debating a PR to add this capability as I agree with the @markzhai that it would be good to add because I personally built something similar to this in our own application because we found Class and field level caching to be beneficial, especially on certain JVM's.

The only difference between my implementation and the suggested one is that we used a ConcurrentHashMap for both the classes and the separate one for the set of fields.

@lukaseder
Copy link
Member

@rdifrango Quite possibly, this might speed up things - but I'd like to be sure about it.

Another caveat that has not been discussed yet is OSGi or other scenarios where caching reflection information will be detrimental. It should certainly be possible to turn off caching, which makes me wonder if we perhaps shouldn't re-design jOOR and avoid the static API... That way, an SPI could be created which allows for injecting different caching strategies.

@rdifrango
Copy link

@lukaseder We aren't in an OSGi environment so it's a good call that it might be detrimental there. I can tell you though in traditional environments the caching definitely is/was beneficial.

I do like the idea of a re-design of the API to avoid the static's allowing for a caching strategy to be customized by users of the library.

@lukaseder
Copy link
Member

Will think about it...

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

No branches or pull requests

3 participants