Skip to content

fix: Use WeakHashMap for caching proxy classes #2260

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

Merged
merged 14 commits into from
Jan 20, 2025
Merged
21 changes: 18 additions & 3 deletions src/main/java/io/appium/java_client/proxy/Helpers.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@

package io.appium.java_client.proxy;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import java.util.Map;
import java.util.WeakHashMap;
import lombok.Value;
import net.bytebuddy.ByteBuddy;
import net.bytebuddy.description.method.MethodDescription;
Expand All @@ -32,14 +35,15 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static java.util.Objects.requireNonNull;
import static net.bytebuddy.matcher.ElementMatchers.namedOneOf;

/**
* The type Helpers.
*/
public class Helpers {
public static final Set<String> OBJECT_METHOD_NAMES = Stream.of(Object.class.getMethods())
.map(Method::getName)
Expand All @@ -50,7 +54,18 @@ public class Helpers {
// the performance and to avoid extensive memory usage for our case, where
// the amount of instrumented proxy classes we create is low in comparison to the amount
// of proxy instances.
private static final ConcurrentMap<ProxyClassSignature, Class<?>> CACHED_PROXY_CLASSES = new ConcurrentHashMap<>();
private static final Map<ProxyClassSignature, Class<?>> CACHED_PROXY_CLASSES = Collections.synchronizedMap(new WeakHashMap<>());

/**
* Gets CACHED_PROXY_CLASSES size.
* Used for cache clear up tests {@see io.appium.java_client.pagefactory_tests.widget.tests.combined.CombinedWidgetTest}.
*
* @return the cached proxy classes size
*/
@VisibleForTesting
public static int getCachedProxyClassesSize() {
return CACHED_PROXY_CLASSES.size();
}

private Helpers() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.appium.java_client.pagefactory_tests.widget.tests.AbstractStubWebDriver;
import io.appium.java_client.pagefactory_tests.widget.tests.DefaultStubWidget;
import io.appium.java_client.pagefactory_tests.widget.tests.android.DefaultAndroidWidget;
import io.appium.java_client.proxy.Helpers;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
Expand All @@ -20,6 +21,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.lessThan;
import static org.openqa.selenium.support.PageFactory.initElements;


Expand Down Expand Up @@ -57,6 +59,7 @@ public static Stream<Arguments> data() {
@ParameterizedTest
@MethodSource("data")
void checkThatWidgetsAreCreatedCorrectly(AbstractApp app, WebDriver driver, Class<?> widgetClass) {
assertProxyClassCacheGrowth();
initElements(new AppiumFieldDecorator(driver), app);
assertThat("Expected widget class was " + widgetClass.getName(),
app.getWidget().getSubWidget().getSelfReference().getClass(),
Expand Down Expand Up @@ -161,4 +164,22 @@ public List<PartiallyCombinedWidget> getWidgets() {
return multipleWidgets;
}
}


/**
* Assert proxy class cache growth for this test class.
* The (@link io.appium.java_client.proxy.Helpers#CACHED_PROXY_CLASSES) should be populated during these tests.
* Prior to the Caching issue being resolved
* - the CACHED_PROXY_CLASSES would grow indefinitely, resulting in an Out Of Memory exception.
* - this ParameterizedTest would have the CACHED_PROXY_CLASSES grow to 266 entries.
*/
private void assertProxyClassCacheGrowth() {
System.gc(); //Trying to force a collection for more accurate check numbers
int thresholdSize = 50;
assertThat(
"Proxy Class Cache threshold is " + thresholdSize,
Helpers.getCachedProxyClassesSize(),
lessThan(thresholdSize)
);
}
}
Loading