azagrebin commented on a change in pull request #11109:
URL: https://github.com/apache/flink/pull/11109#discussion_r416409014
##########
File path:
flink-core/src/main/java/org/apache/flink/util/JavaGcCleanerWrapper.java
##########
@@ -43,326 +47,338 @@
private static final Logger LOG =
LoggerFactory.getLogger(JavaGcCleanerWrapper.class);
private static final Collection<CleanerProvider> CLEANER_PROVIDERS =
- Arrays.asList(LegacyCleanerProvider.INSTANCE,
Java9CleanerProvider.INSTANCE);
- private static final CleanerFactory CLEANER_FACTORY = findGcCleaner();
+ Arrays.asList(createLegacyCleanerProvider(),
createJava9CleanerProvider());
+ private static final CleanerManager CLEANER_MANAGER =
findGcCleanerManager();
+
+ private static CleanerProvider createLegacyCleanerProvider() {
+ String name = "Legacy (before Java 9) cleaner";
+ ReflectionUtils reflectionUtils = new ReflectionUtils(name + "
provider");
+ String cleanerClassName = "sun.misc.Cleaner";
+ return new CleanerProvider(
+ name,
+ new CleanerFactoryProvider(
+ name,
+ reflectionUtils,
+ cleanerClassName,
+ Optional::empty, // there is no Cleaner object,
static method of its class will be called to create it
+ "create", // static method of Cleaner class to
create it
+ cleanerClassName, // Cleaner is Cleanable in
this case
+ "clean"),
+ new PendingCleanersRunnerProvider(
+ name,
+ reflectionUtils,
+ "sun.misc.SharedSecrets",
+ "sun.misc.JavaLangRefAccess",
+ "getJavaLangRefAccess",
+ "tryHandlePendingReference"));
+ }
+
+ private static CleanerProvider createJava9CleanerProvider() {
+ String name = "New Java 9+ cleaner";
+ ReflectionUtils reflectionUtils = new ReflectionUtils(name + "
provider");
+ String cleanerClassName = "java.lang.ref.Cleaner";
+ return new CleanerProvider(
+ name,
+ new CleanerFactoryProvider(
+ name,
+ reflectionUtils,
+ cleanerClassName,
+ () -> {
+ Class<?> cleanerClass =
reflectionUtils.findClass(cleanerClassName);
+ Method cleanerCreateMethod =
reflectionUtils.findMethod(cleanerClass, "create");
+ try {
+ return
Optional.of(cleanerCreateMethod.invoke(null));
+ } catch (IllegalAccessException |
InvocationTargetException e) {
+ throw new
FlinkRuntimeException("Failed to create a Java 9 Cleaner", e);
+ }
+ },
+ "register",
+ "java.lang.ref.Cleaner$Cleanable",
+ "clean"),
+ new PendingCleanersRunnerProvider(
+ name,
+ reflectionUtils,
+ "jdk.internal.misc.SharedSecrets",
+ "jdk.internal.misc.JavaLangRefAccess",
+ "getJavaLangRefAccess",
+ "waitForReferenceProcessing"));
+ }
- private static CleanerFactory findGcCleaner() {
- CleanerFactory foundCleanerFactory = null;
+ private static CleanerManager findGcCleanerManager() {
+ CleanerManager foundCleanerManager = null;
Throwable t = null;
for (CleanerProvider cleanerProvider : CLEANER_PROVIDERS) {
- //noinspection OverlyBroadCatchBlock
try {
- foundCleanerFactory =
cleanerProvider.createCleanerFactory();
+ foundCleanerManager =
cleanerProvider.createCleanerManager();
break;
} catch (Throwable e) {
t = ExceptionUtils.firstOrSuppressed(e, t);
}
}
- if (foundCleanerFactory == null) {
+ if (foundCleanerManager == null) {
String errorMessage = String.format("Failed to find GC
Cleaner among available providers: %s", CLEANER_PROVIDERS);
throw new Error(errorMessage, t);
}
- return foundCleanerFactory;
+ return foundCleanerManager;
}
public static Runnable create(Object owner, Runnable cleanOperation) {
- return CLEANER_FACTORY.create(owner, cleanOperation);
+ return CLEANER_MANAGER.create(owner, cleanOperation);
}
- public static boolean waitForGcToRunReadyCleaners() throws
InterruptedException {
- return CLEANER_FACTORY.waitForGcToRunReadyCleaners();
+ public static boolean tryRunPendingCleaners() throws
InterruptedException {
+ return CLEANER_MANAGER.tryRunPendingCleaners();
}
- private static Class<?> findClass(String className, String
errorMessage) {
- try {
- return Class.forName(className);
- } catch (ClassNotFoundException e) {
- throw new FlinkRuntimeException(errorMessage, e);
+ private static class CleanerProvider {
+ private final String cleanerName;
+ private final CleanerFactoryProvider cleanerFactoryProvider;
+ private final PendingCleanersRunnerProvider
pendingCleanersRunnerProvider;
+
+ private CleanerProvider(
+ String cleanerName,
+ CleanerFactoryProvider cleanerFactoryProvider,
+ PendingCleanersRunnerProvider
pendingCleanersRunnerProvider) {
+ this.cleanerName = cleanerName;
+ this.cleanerFactoryProvider = cleanerFactoryProvider;
+ this.pendingCleanersRunnerProvider =
pendingCleanersRunnerProvider;
}
- }
-
- @FunctionalInterface
- private interface CleanerProvider {
- CleanerFactory createCleanerFactory() throws
ClassNotFoundException;
- }
- private interface CleanerFactory {
- Runnable create(Object owner, Runnable cleanOperation);
+ private CleanerManager createCleanerManager() {
+ return new CleanerManager(
+ cleanerName,
+ cleanerFactoryProvider.createCleanerFactory(),
+
pendingCleanersRunnerProvider.createPendingCleanersRunner());
+ }
- boolean waitForGcToRunReadyCleaners() throws
InterruptedException;
+ @Override
+ public String toString() {
+ return cleanerName + " provider";
+ }
}
- private enum LegacyCleanerProvider implements CleanerProvider {
- INSTANCE;
-
- private static final String LEGACY_CLEANER_CLASS_NAME =
"sun.misc.Cleaner";
- private static final String LEGACY_SHARED_SECRETS =
"sun.misc.SharedSecrets";
- private static final String LEGACY_JAVA_LANG_REF_ACCESS =
"sun.misc.JavaLangRefAccess";
+ private static class CleanerManager {
+ private final String cleanerName;
+ private final CleanerFactory cleanerFactory;
+ private final PendingCleanersRunner pendingCleanersRunner;
+
+ private CleanerManager(
+ String cleanerName,
+ CleanerFactory cleanerFactory,
+ PendingCleanersRunner pendingCleanersRunner) {
+ this.cleanerName = cleanerName;
+ this.cleanerFactory = cleanerFactory;
+ this.pendingCleanersRunner = pendingCleanersRunner;
+ }
- @Override
- public CleanerFactory createCleanerFactory() {
- Class<?> cleanerClass =
findClass(LEGACY_CLEANER_CLASS_NAME, "Failed to find Java legacy Cleaner
class");
- Class<?> sharedSecretsClass =
findClass(LEGACY_SHARED_SECRETS, "Failed to find Java legacy SharedSecrets
class");
- Class<?> javaLangRefAccessClass =
findClass(LEGACY_JAVA_LANG_REF_ACCESS, "Failed to find Java legacy
JavaLangRefAccess class");
- Method cleanerCreateMethod =
getCleanerCreateMethod(cleanerClass);
- Method cleanerCleanMethod =
getCleanerCleanMethod(cleanerClass);
- Method getJavaLangRefAccessMethod =
getSharedSecretsGetJavaLangRefAccessMethod(sharedSecretsClass);
- Method tryHandlePendingReferenceMethod =
getJavaLangRefAccessTryHandlePendingReference(javaLangRefAccessClass);
- return new LegacyCleanerFactory(cleanerCreateMethod,
cleanerCleanMethod, getJavaLangRefAccessMethod,
tryHandlePendingReferenceMethod);
+ private Runnable create(Object owner, Runnable cleanOperation) {
+ return cleanerFactory.create(owner, cleanOperation);
}
- private static Method getCleanerCreateMethod(Class<?>
cleanerClass) {
- try {
- return cleanerClass.getMethod("create",
Object.class, Runnable.class);
- } catch (NoSuchMethodException e) {
- throw new FlinkRuntimeException("Failed to find
Java legacy Cleaner#create method", e);
- }
+ private boolean tryRunPendingCleaners() throws
InterruptedException {
+ return pendingCleanersRunner.tryRunPendingCleaners();
}
- private static Method getCleanerCleanMethod(Class<?>
cleanerClass) {
- try {
- return cleanerClass.getMethod("clean");
- } catch (NoSuchMethodException e) {
- throw new FlinkRuntimeException("Failed to find
Java legacy Cleaner#clean method", e);
- }
+ @Override
+ public String toString() {
+ return cleanerName + " manager";
}
+ }
- private static Method
getSharedSecretsGetJavaLangRefAccessMethod(Class<?> sharedSecretsClass) {
- try {
- return
sharedSecretsClass.getMethod("getJavaLangRefAccess");
- } catch (NoSuchMethodException e) {
- throw new FlinkRuntimeException("Failed to find
Java legacy SharedSecrets#getJavaLangRefAccess method", e);
- }
+ private static class CleanerFactoryProvider {
+ private final String cleanerName;
+ private final ReflectionUtils reflectionUtils;
+ private final String cleanerClassName;
+ private final Supplier<Optional<Object>> cleanerSupplier;
+ private final String cleanableCreationMethodName;
+ private final String cleanableClassName;
+ private final String cleanMethodName;
+
+ private CleanerFactoryProvider(
+ String cleanerName,
+ ReflectionUtils reflectionUtils,
+ String cleanerClassName,
+ Supplier<Optional<Object>> cleanerSupplier,
+ String cleanableCreationMethodName, // Cleaner
is a factory for Cleanable
+ String cleanableClassName,
+ String cleanMethodName) {
+ this.cleanerName = cleanerName;
+ this.reflectionUtils = reflectionUtils;
+ this.cleanerClassName = cleanerClassName;
+ this.cleanerSupplier = cleanerSupplier;
+ this.cleanableCreationMethodName =
cleanableCreationMethodName;
+ this.cleanableClassName = cleanableClassName;
+ this.cleanMethodName = cleanMethodName;
}
- private static Method
getJavaLangRefAccessTryHandlePendingReference(Class<?> javaLangRefAccessClass) {
- try {
- return
javaLangRefAccessClass.getMethod("tryHandlePendingReference");
- } catch (NoSuchMethodException e) {
- throw new FlinkRuntimeException("Failed to find
Java legacy JavaLangRefAccess#tryHandlePendingReference method", e);
- }
+ private CleanerFactory createCleanerFactory() {
+ Class<?> cleanerClass =
reflectionUtils.findClass(cleanerClassName);
+ Method cleanableCreationMethod =
reflectionUtils.findMethod(
+ cleanerClass,
+ cleanableCreationMethodName,
+ Object.class,
+ Runnable.class);
+ Class<?> cleanableClass =
reflectionUtils.findClass(cleanableClassName);
+ Method cleanMethod =
reflectionUtils.findMethod(cleanableClass, cleanMethodName);
+ return new CleanerFactory(
+ cleanerName,
+ cleanerSupplier.get().orElse(null), // static
method of Cleaner class will be called to create Cleanable
+ cleanableCreationMethod,
+ cleanMethod);
}
@Override
public String toString() {
- return "Legacy cleaner provider before Java 9 using " +
LEGACY_CLEANER_CLASS_NAME;
+ return cleanerName + " factory provider using " +
cleanerClassName;
}
}
- private static final class LegacyCleanerFactory implements
CleanerFactory {
- private final Method cleanerCreateMethod;
- private final Method cleanerCleanMethod;
- private final Method getJavaLangRefAccessMethod;
- private final Method tryHandlePendingReferenceMethod;
-
- private LegacyCleanerFactory(
- Method cleanerCreateMethod,
- Method cleanerCleanMethod,
- Method getJavaLangRefAccessMethod,
- Method tryHandlePendingReferenceMethod) {
- this.cleanerCreateMethod = cleanerCreateMethod;
- this.cleanerCleanMethod = cleanerCleanMethod;
- this.getJavaLangRefAccessMethod =
getJavaLangRefAccessMethod;
- this.tryHandlePendingReferenceMethod =
tryHandlePendingReferenceMethod;
+ private static class CleanerFactory {
Review comment:
Similar thoughts came into my mind. I think both approaches have
benefits and drawbacks. Having less code makes it fast to understand what
generally happens in both cases. True, arguably, the difference may be more
obvious if you just compare two pieces of code. For this reason, I left
comments for Java 8 case explaining the difference. It is also a good idea to
add the normal Java code for both cases, I can do this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
