Unwrapped java.sql PreparedStatement and Connection

[randomanderson]

java.sql.PreparedStatement and java.sql.Connection are wrappers to delegates when they implement the Wrapper interface. When this happens, our previous instrumentation was unable to find the saved sql statements in the ContextStore. This pull request unwraps these objects to do lookups on the delegates instead of the wrappers.

The one complication is older drivers like h2 implementing an older version of the interface. This throws AbstractMethodError at runtime. I cached which classes throw the error for efficiency. I added a couple methods to DDCache so it can be used in the implementation.

[richardstartin]

Could we use ClassValue here instead? (which have weak keys)

[richardstartin]

These changes aren't necessary if we, ahem, wrap the unwrapping logic in a ClassValue<Boolean>

[richardstartin]

The majority of this change seems to be caching changes for caching flags at class level, whereas I think this is a classic use case for ClassValue, which is well optimised for this sort of thing.

[randomanderson]

The majority of this change seems to be caching changes for caching flags at class level, whereas I think this is a classic use case for ClassValue, which is well optimised for this sort of thing.

@richardstartin The main reason for those changes was the only way to add items to a DDCache was through computeIfAbsent. ClassValue has the same limitation.

Why this matters:
Any jdbc library written after jdk6 will have the isWrappedFor/unwrap methods available. I think this represents the vast majority of libraries in use.

With the way the PR is currently written, the common case just works. We catch the extremely rare AbstractMethodError and cache that so we know to skip that class in the future.

In a computeIfAbsent approach, we have to check for the availability of the methods in all libraries.

I guess it the end it doesn't really matter because its only once per class extending PreparedStatement/Connection

[richardstartin]

@randomanderson even so, you're going to hit the cache in JDBCUtils.unwrappedStatement for every PreparedStatement type even if it always misses, and I expect we'll find that ClassValue.get is quite a lot lower overhead than any FixedSizeCache operation we can do. Does this lead to storing false against the "normal" PreparedStatement classes? Yes, but I don't think it matters and I would be surprised if there three or more of these per application anyway.

Since I'm claiming a performance advantage for ClassValue, I'm willing to produce some evidence to back that up, but maybe we can talk it through later.

[richardstartin]

I wrote up what I have in mind here

[richardstartin]

It looks like ClassValue<Boolean> has better performance than FixedSizeCache<String, Boolean> at least when the number of types is small (didn't test beyond 7) e.g.

public class ClassPredicate extends ClassValue<Boolean> {

    private final Predicate<Class<?>> predicate;

    public ClassPredicate(Predicate<Class<?>> predicate) {
        this.predicate = predicate;
    }


    @Override
    protected Boolean computeValue(Class<?> type) {
        return predicate.test(type);
    }
}

@State(Scope.Benchmark)
public class ClassValueVsFixedSizeCache {

    private ClassPredicate classPredicate;
    private FixedSizeCache<String, Boolean> fixedSizeCache;

    private Class[] types;
    int i = 0;

    @Setup(Level.Iteration)
    public void init() {
        this.classPredicate = new ClassPredicate(type -> type.getName().equals("java.lang.String"));
        this.fixedSizeCache = new FixedSizeCache<>(64);
        this.types = new Class[]{
                Short.class,
                Long.class,
                Double.class,
                Float.class,
                Object.class,
                Integer.class,
                String.class
        };
    }


    @Benchmark
    public Boolean classPredicate() {
        int index = i;
        i = (i + 1) % 7;
        return classPredicate.get(types[index]);
    }

    @Benchmark
    public Boolean fixedSizeCache() {
        int index = i;
        i = (i + 1) % 7;
        String type = types[index].getName();
        Boolean result = fixedSizeCache.getIfPresent(type);
        if (null == result && index == 6) {
            fixedSizeCache.put(type, true);
            return true;
        }
        return result;
    }
}

ClassValueVsFixedSizeCache.classPredicate  avgt    5  5.614 ± 0.097  ns/op
ClassValueVsFixedSizeCache.fixedSizeCache  avgt    5  9.812 ± 0.276  ns/op

Rather than making a material difference to a user's application, this would probably only show up in CPU% metrics.

[richardstartin]

Nice. Thanks for accommodating the requested changes and fixing my buggy suggestion.