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.

Sign up for GitHub

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

Jump to bottom

Cache MethodHandles in LocalCacheFactory and NodeFactory #905

Merged
merged 6 commits into from
Apr 29, 2023
Merged

Cache MethodHandles in LocalCacheFactory and NodeFactory #905

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
557aa49
Cache constructor MethodHandles in factories
yuzawa-san Apr 10, 2023
b24b5a5
Cache constructor MethodHandles in factories
yuzawa-san Apr 12, 2023
88a9f13
Cache constructor MethodHandles in factories
yuzawa-san Apr 13, 2023
bf64136
Cache BoundedLocalCache and Node factory instances
yuzawa-san Apr 18, 2023
a1aab75
Cache LocalCacheFactory and NodeFactory instances
yuzawa-san Apr 23, 2023
f9cdf54
Merge branch 'master' into factory-cached-constructors
ben-manes Apr 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 30 additions & 10 deletions caffeine/src/jmh/java/com/github/benmanes/caffeine/FactoryBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
*/
package com.github.benmanes.caffeine;

import java.lang.invoke.LambdaMetafactory;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.reflect.Constructor;

import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.infra.Blackhole;

/**
* @author ben.manes@gmail.com (Ben Manes)
Expand All @@ -34,35 +35,46 @@ public class FactoryBenchmark {
private final MethodHandleFactory methodHandleFactory = new MethodHandleFactory();

@Benchmark
public Alpha direct() {
return new Alpha();
public void direct(Blackhole blackhole) {
blackhole.consume(new Alpha());
}

@Benchmark
public Alpha methodHandle_invoke() {
return methodHandleFactory.invoke();
public void methodHandle_invoke(Blackhole blackhole) {
blackhole.consume(methodHandleFactory.invoke());
}

@Benchmark
public Alpha methodHandle_invokeExact() {
return methodHandleFactory.invokeExact();
public void methodHandle_invokeExact(Blackhole blackhole) {
blackhole.consume(methodHandleFactory.invokeExact());
}

@Benchmark
public Alpha reflection() {
return reflectionFactory.newInstance();
public void methodHandle_lambda(Blackhole blackhole) {
blackhole.consume(methodHandleFactory.lambda());
}

@Benchmark
public void reflection(Blackhole blackhole) {
blackhole.consume(reflectionFactory.newInstance());
}

static final class MethodHandleFactory {
private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
private static final MethodType METHOD_TYPE = MethodType.methodType(void.class);

private final MethodHandle methodHandle;
private final AlphaConstructor lambda;

MethodHandleFactory() {
try {
methodHandle = LOOKUP.findConstructor(Alpha.class, METHOD_TYPE);
} catch (NoSuchMethodException | IllegalAccessException e) {
lambda =
(AlphaConstructor) LambdaMetafactory
.metafactory(LOOKUP, "construct", MethodType.methodType(AlphaConstructor.class),
methodHandle.type(), methodHandle, methodHandle.type())
.getTarget().invokeExact();
} catch (Throwable e) {
throw new RuntimeException(e);
}
}
Expand All @@ -82,6 +94,10 @@ Alpha invokeExact() {
throw new RuntimeException(e);
}
}

Alpha lambda() {
return lambda.construct();
}
}

static final class ReflectionFactory {
Expand All @@ -107,4 +123,8 @@ Alpha newInstance() {
static final class Alpha {
public Alpha() {}
}

private interface AlphaConstructor {
Alpha construct();
}
}
22 changes: 18 additions & 4 deletions caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalCacheFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import org.checkerframework.checker.nullness.qual.Nullable;

Expand All @@ -30,6 +32,7 @@ final class LocalCacheFactory {
private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
private static final MethodType FACTORY = MethodType.methodType(
void.class, Caffeine.class, AsyncCacheLoader.class, boolean.class);
private static final Map<String, MethodHandle> CONSTRUCTORS = new ConcurrentHashMap<>();

private LocalCacheFactory() {}

Expand All @@ -41,7 +44,7 @@ static <K, V> BoundedLocalCache<K, V> newBoundedLocalCache(Caffeine<K, V> builde
}

static String getClassName(Caffeine<?, ?> builder) {
var className = new StringBuilder(LocalCacheFactory.class.getPackageName()).append('.');
var className = new StringBuilder();
if (builder.isStrongKeys()) {
className.append('S');
} else {
Expand Down Expand Up @@ -80,10 +83,21 @@ static String getClassName(Caffeine<?, ?> builder) {

static <K, V> BoundedLocalCache<K, V> loadFactory(Caffeine<K, V> builder,
@Nullable AsyncCacheLoader<? super K, V> cacheLoader, boolean async, String className) {
var constructor = CONSTRUCTORS.get(className);
if (constructor == null) {
constructor = CONSTRUCTORS.computeIfAbsent(className, LocalCacheFactory::newConstructor);
}
try {
return (BoundedLocalCache<K, V>) constructor.invoke(builder, cacheLoader, async);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use invokeExact here after changing the thing below.

} catch (Throwable t) {
throw new IllegalStateException(className, t);
}
}

static MethodHandle newConstructor(String className) {
try {
Class<?> clazz = Class.forName(className);
MethodHandle handle = LOOKUP.findConstructor(clazz, FACTORY);
return (BoundedLocalCache<K, V>) handle.invoke(builder, cacheLoader, async);
Class<?> clazz = Class.forName(LocalCacheFactory.class.getPackageName() + "." + className);
return LOOKUP.findConstructor(clazz, FACTORY);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to: return LOOKUP.findConstructor(clazz, FACTORY).changeReturnType(BoundedLocalCache.class)

This will adapt the return type, so the above call will have correct typoeninformation and invokeExact works.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One additional thing: Since Java 11 I'd use: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#findClass(java.lang.String)

So instead of a plain Class.forName() you can use LOOKUP.findClass(), so both fits together much better. This uses the accessibility of the used Lookup and so it ensure that the class is visible and can be loaded. It also simplifies access checks.

} catch (RuntimeException | Error e) {
throw e;
} catch (Throwable t) {
Expand Down
22 changes: 18 additions & 4 deletions caffeine/src/main/java/com/github/benmanes/caffeine/cache/NodeFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.ref.ReferenceQueue;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import com.github.benmanes.caffeine.cache.References.LookupKeyReference;
import com.github.benmanes.caffeine.cache.References.WeakKeyReference;
Expand All @@ -31,6 +33,7 @@
interface NodeFactory<K, V> {
MethodType FACTORY = MethodType.methodType(void.class);
MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
Map<String, MethodHandle> CONSTRUCTORS = new ConcurrentHashMap<>();

RetiredStrongKey RETIRED_STRONG_KEY = new RetiredStrongKey();
RetiredWeakKey RETIRED_WEAK_KEY = new RetiredWeakKey();
Expand Down Expand Up @@ -87,7 +90,7 @@ static <K, V> NodeFactory<K, V> newFactory(Caffeine<K, V> builder, boolean isAsy
}

static String getClassName(Caffeine<?, ?> builder, boolean isAsync) {
var className = new StringBuilder(Node.class.getPackageName()).append('.');
var className = new StringBuilder();
if (builder.isStrongKeys()) {
className.append('P');
} else {
Expand Down Expand Up @@ -132,10 +135,21 @@ static String getClassName(Caffeine<?, ?> builder, boolean isAsync) {
}

static <K, V> NodeFactory<K, V> loadFactory(String className) {
var constructor = CONSTRUCTORS.get(className);
if (constructor == null) {
constructor = CONSTRUCTORS.computeIfAbsent(className, NodeFactory::newConstructor);
}
try {
return (NodeFactory<K, V>) constructor.invoke();
} catch (Throwable t) {
throw new IllegalStateException(className, t);
}
}

static MethodHandle newConstructor(String className) {
try {
Class<?> clazz = Class.forName(className);
MethodHandle handle = LOOKUP.findConstructor(clazz, FACTORY);
return (NodeFactory<K, V>) handle.invoke();
Class<?> clazz = Class.forName(Node.class.getPackageName() + "." + className);
return LOOKUP.findConstructor(clazz, FACTORY);
} catch (RuntimeException | Error e) {
throw e;
} catch (Throwable t) {
Expand Down
1 change: 0 additions & 1 deletion caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
Expand Down