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

Metaspace improvements in QuarkusUnitTest (and dev mode!) - round 3 #36560

Merged
merged 11 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@
import java.util.function.IntFunction;
import java.util.regex.Pattern;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.spi.ConfigBuilder;
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.eclipse.microprofile.config.spi.Converter;
import org.objectweb.asm.Opcodes;
import org.wildfly.common.Assert;
Expand Down Expand Up @@ -123,13 +121,6 @@ public final class RunTimeConfigurationGenerator {
static final MethodDescriptor CONVS_PATTERN_CONVERTER = MethodDescriptor.ofMethod(Converters.class,
"patternConverter", Converter.class, Converter.class, Pattern.class);

static final MethodDescriptor CPR_GET_CONFIG = MethodDescriptor.ofMethod(ConfigProviderResolver.class, "getConfig",
Config.class);
static final MethodDescriptor CPR_INSTANCE = MethodDescriptor.ofMethod(ConfigProviderResolver.class, "instance",
ConfigProviderResolver.class);
static final MethodDescriptor CPR_RELEASE_CONFIG = MethodDescriptor.ofMethod(ConfigProviderResolver.class, "releaseConfig",
void.class, Config.class);

static final MethodDescriptor CU_LIST_FACTORY = MethodDescriptor.ofMethod(ConfigUtils.class, "listFactory",
IntFunction.class);
static final MethodDescriptor CU_SET_FACTORY = MethodDescriptor.ofMethod(ConfigUtils.class, "setFactory",
Expand Down Expand Up @@ -595,17 +586,7 @@ private Set<String> getRegisteredRoots(ConfigPhase configPhase) {
}

private void installConfiguration(ResultHandle config, MethodCreator methodCreator) {
// install config
methodCreator.invokeStaticMethod(QCF_SET_CONFIG, config);
// now invalidate the cached config, so the next one to load the config gets the new one
final ResultHandle configProviderResolver = methodCreator.invokeStaticMethod(CPR_INSTANCE);
try (TryBlock getConfigTry = methodCreator.tryBlock()) {
final ResultHandle initialConfigHandle = getConfigTry.invokeVirtualMethod(CPR_GET_CONFIG,
configProviderResolver);
getConfigTry.invokeVirtualMethod(CPR_RELEASE_CONFIG, configProviderResolver, initialConfigHandle);
// ignore
getConfigTry.addCatch(IllegalStateException.class);
}
}

private MethodDescriptor generateInitGroup(ClassDefinition definition) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.function.Consumer;
import java.util.function.Predicate;

import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.jboss.logging.Logger;
import org.jboss.logmanager.formatters.ColorPatternFormatter;
import org.jboss.logmanager.handlers.ConsoleHandler;
Expand Down Expand Up @@ -284,26 +283,22 @@ public void stop() {
ClassLoader old = Thread.currentThread().getContextClassLoader();
Thread.currentThread().setContextClassLoader(runner.getClassLoader());
try {
runner.close();
} catch (Exception e) {
e.printStackTrace();
try {
runner.close();
} catch (Exception e) {
e.printStackTrace();
}
try {
// TODO is this even useful?
// It's not using the config factory from the right classloader...
QuarkusConfigFactory.setConfig(null);
} catch (Exception e) {
e.printStackTrace();
}
} finally {
Thread.currentThread().setContextClassLoader(old);
}
}
QuarkusConfigFactory.setConfig(null);

ClassLoader old = Thread.currentThread().getContextClassLoader();
Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
try {
final ConfigProviderResolver cpr = ConfigProviderResolver.instance();
cpr.releaseConfig(cpr.getConfig());
} catch (Throwable ignored) {
// just means no config was installed, which is fine
} finally {
Thread.currentThread().setContextClassLoader(old);
}
runner = null;
}

public void close() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ public void init() {
for (ResolvedDependency d : testModel.getDependencies()) {
if (d.isClassLoaderParentFirst() && !currentParentFirst.contains(d.getKey())) {
if (clBuilder == null) {
clBuilder = QuarkusClassLoader.builder("Continuous Testing Parent-First",
clBuilder = QuarkusClassLoader.builder("Continuous Testing Parent-First"
+ curatedApplication.getClassLoaderNameSuffix(),
getClass().getClassLoader().getParent(), false);
}
clBuilder.addElement(ClassPathElement.fromDependency(d));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import io.quarkus.deployment.builditem.QuarkusBuildCloseablesBuildItem;
import io.quarkus.deployment.builditem.RunTimeConfigBuilderBuildItem;
import io.quarkus.deployment.builditem.RunTimeConfigurationDefaultBuildItem;
import io.quarkus.deployment.builditem.ShutdownContextBuildItem;
import io.quarkus.deployment.builditem.StaticInitConfigBuilderBuildItem;
import io.quarkus.deployment.builditem.SuppressNonRuntimeConfigChangedWarningBuildItem;
import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
Expand Down Expand Up @@ -305,6 +306,13 @@ public void suppressNonRuntimeConfigChanged(
suppressNonRuntimeConfigChanged.produce(new SuppressNonRuntimeConfigChangedWarningBuildItem("quarkus.test.arg-line"));
}

@BuildStep
@Record(ExecutionTime.RUNTIME_INIT)
public void releaseConfigOnShutdown(ShutdownContextBuildItem shutdownContext,
ConfigRecorder recorder) {
recorder.releaseConfig(shutdownContext);
}

/**
* Warns if build time config properties have been changed at runtime.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ private void doClose() {

@Override
public int runMainClassBlocking(String... args) throws Exception {
//first we hack around class loading in the fork join pool
ForkJoinClassLoading.setForkJoinClassLoader(runtimeClassLoader);

//we have our class loaders
ClassLoader old = Thread.currentThread().getContextClassLoader();
Thread.currentThread().setContextClassLoader(runtimeClassLoader);
Expand Down Expand Up @@ -252,7 +255,7 @@ public void overrideConfig(Map<String, String> config) {
* Runs the application, and returns a handle that can be used to shut it down.
*/
public RunningQuarkusApplication run(String... args) throws Exception {
//first
//first we hack around class loading in the fork join pool
ForkJoinClassLoading.setForkJoinClassLoader(runtimeClassLoader);

//we have our class loaders
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.eclipse.microprofile.config.spi.ConfigSource;
import org.jboss.logging.Logger;

import io.quarkus.runtime.ShutdownContext;
import io.quarkus.runtime.annotations.Recorder;
import io.quarkus.runtime.configuration.ConfigurationRuntimeConfig.BuildTimeMismatchAtRuntime;
import io.smallrye.config.SmallRyeConfig;
Expand Down Expand Up @@ -101,4 +102,12 @@ public void handleNativeProfileChange(List<String> buildProfiles) {
public void unknownConfigFiles() throws Exception {
ConfigDiagnostic.unknownConfigFiles(ConfigDiagnostic.configFilesFromLocations());
}

public void releaseConfig(ShutdownContext shutdownContext) {
// This is mostly useful to handle restarts in Dev/Test mode.
// While this may seem to duplicate code in IsolatedDevModeMain,
// it actually does not because it operates on a different instance
// of QuarkusConfigFactory from a different classloader.
shutdownContext.addLastShutdownTask(QuarkusConfigFactory::releaseTCCLConfig);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,39 @@ public QuarkusConfigFactory() {
// todo: replace with {@code provider()} post-Java 11
}

@Override
public SmallRyeConfig getConfigFor(final SmallRyeConfigProviderResolver configProviderResolver,
final ClassLoader classLoader) {
if (config == null) {
return ConfigUtils.configBuilder(true, true, LaunchMode.NORMAL).build();
// Remember the config so that we can uninstall it when "setConfig" is next called
config = ConfigUtils.configBuilder(true, true, LaunchMode.NORMAL).build();
}
return config;
}

public static void setConfig(SmallRyeConfig config) {
SmallRyeConfigProviderResolver configProviderResolver = (SmallRyeConfigProviderResolver) SmallRyeConfigProviderResolver
.instance();
configProviderResolver.releaseConfig(Thread.currentThread().getContextClassLoader());
QuarkusConfigFactory.config = config;
// Uninstall previous config
if (QuarkusConfigFactory.config != null) {
configProviderResolver.releaseConfig(QuarkusConfigFactory.config);
radcortez marked this conversation as resolved.
Show resolved Hide resolved
QuarkusConfigFactory.config = null;
}
// Also release the TCCL config, in case that config was not QuarkusConfigFactory.config
configProviderResolver.releaseConfig(Thread.currentThread().getContextClassLoader());
// Install new config
if (config != null) {
QuarkusConfigFactory.config = config;
// Register the new config for the TCCL,
// just in case the TCCL was using a different config
// than the one we uninstalled above.
configProviderResolver.registerConfig(config, Thread.currentThread().getContextClassLoader());
}
}

public static void releaseTCCLConfig() {
SmallRyeConfigProviderResolver configProviderResolver = (SmallRyeConfigProviderResolver) SmallRyeConfigProviderResolver
.instance();
configProviderResolver.releaseConfig(Thread.currentThread().getContextClassLoader());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,10 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigProvider;
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import io.quarkus.runtime.configuration.QuarkusConfigFactory;
import io.smallrye.config.SmallRyeConfig;
import io.smallrye.config.SmallRyeConfigBuilder;

Expand All @@ -21,34 +17,26 @@
*/
public class ApplicationYamlTest {

static volatile SmallRyeConfig config;
private static SmallRyeConfig config;

@BeforeAll
public static void doBefore() {
SmallRyeConfigBuilder builder = new SmallRyeConfigBuilder();
builder.addDefaultSources().addDiscoveredConverters().addDiscoveredSources();
QuarkusConfigFactory.setConfig(config = builder.build());
Config conf = ConfigProvider.getConfig();
if (conf != config) {
ConfigProviderResolver cpr = ConfigProviderResolver.instance();
cpr.releaseConfig(conf);
ConfigProvider.getConfig();
}
config = builder.build();
System.out.println(System.getProperty("user.dir"));
}

@Test
public void testBasicApplicationYaml() {
assertEquals("something", ConfigProvider.getConfig().getValue("foo.bar", String.class));
assertEquals("somethingElse", ConfigProvider.getConfig().getValue("foo2.bar", String.class));
assertEquals("other", ConfigProvider.getConfig().getValue("foo.baz", String.class));
assertTrue(ConfigProvider.getConfig().getValue("file.system", Boolean.class).booleanValue());
assertEquals("something", config.getValue("foo.bar", String.class));
assertEquals("somethingElse", config.getValue("foo2.bar", String.class));
assertEquals("other", config.getValue("foo.baz", String.class));
assertTrue(config.getValue("file.system", Boolean.class).booleanValue());
}

@AfterAll
public static void doAfter() {
ConfigProviderResolver cpr = ConfigProviderResolver.instance();
cpr.releaseConfig(config);
config = null;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
quarkus.datasource.users.db-kind=h2
quarkus.datasource.users.jdbc.url=jdbc:h2:tcp://localhost/mem:users
quarkus.datasource.users.jdbc.url=jdbc:h2:mem:users

quarkus.hibernate-orm."users".log.sql=true
quarkus.hibernate-orm."users".database.generation=drop-and-create
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,38 +6,19 @@
import java.net.URL;
import java.util.Optional;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigProvider;
import org.eclipse.microprofile.config.spi.ConfigBuilder;
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.eclipse.microprofile.rest.client.ext.QueryParamStyle;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import io.smallrye.config.PropertiesConfigSource;
import io.smallrye.config.SmallRyeConfig;
import io.smallrye.config.SmallRyeConfigBuilder;

public class RestClientConfigTest {
yrodiere marked this conversation as resolved.
Show resolved Hide resolved

private static ClassLoader classLoader;
private static ConfigProviderResolver configProviderResolver;

@BeforeAll
public static void initConfig() {
classLoader = Thread.currentThread().getContextClassLoader();
configProviderResolver = ConfigProviderResolver.instance();
}

@AfterEach
public void releaseConfig() {
configProviderResolver.releaseConfig(configProviderResolver.getConfig());
}

@Test
public void testLoadRestClientConfig() throws IOException {
setupMPConfig();
SmallRyeConfig config = createMPConfig();

Config config = ConfigProvider.getConfig();
Optional<String> optionalValue = config.getOptionalValue("quarkus.rest-client.test-client.url", String.class);
assertThat(optionalValue).isPresent();

Expand Down Expand Up @@ -75,11 +56,11 @@ private void verifyConfig(RestClientConfig config) {
assertThat(config.multipart.maxChunkSize.get()).isEqualTo(1024);
}

private static void setupMPConfig() throws IOException {
ConfigBuilder configBuilder = configProviderResolver.getBuilder();
private static SmallRyeConfig createMPConfig() throws IOException {
SmallRyeConfigBuilder configBuilder = new SmallRyeConfigBuilder().addDefaultInterceptors();
URL propertyFile = RestClientConfigTest.class.getClassLoader().getResource("application.properties");
assertThat(propertyFile).isNotNull();
configBuilder.withSources(new PropertiesConfigSource(propertyFile));
configProviderResolver.registerConfig(configBuilder.build(), classLoader);
return configBuilder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,10 @@
import jakarta.ws.rs.client.ClientResponseContext;
import jakarta.ws.rs.client.ClientResponseFilter;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.eclipse.microprofile.rest.client.RestClientBuilder;
import org.eclipse.microprofile.rest.client.ext.QueryParamStyle;
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
Expand All @@ -42,7 +39,6 @@ public class RestClientBaseTest {

private static Path truststorePath;
private static Path keystorePath;
private static Config createdConfig;

@BeforeAll
public static void beforeAll() throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException {
Expand All @@ -65,14 +61,6 @@ public static void beforeAll() throws IOException, KeyStoreException, Certificat
}
}

@AfterEach
public void afterEach() {
if (createdConfig != null) {
ConfigProviderResolver.instance().releaseConfig(createdConfig);
createdConfig = null;
}
}

@AfterAll
public static void afterAll() {
if (truststorePath != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,9 @@
import jakarta.ws.rs.client.ClientResponseContext;
import jakarta.ws.rs.client.ClientResponseFilter;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.eclipse.microprofile.rest.client.ext.QueryParamStyle;
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
import org.jboss.resteasy.reactive.client.api.QuarkusRestClientProperties;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
Expand All @@ -42,7 +39,6 @@ public class RestClientCDIDelegateBuilderTest {
static File tempDir;
private static File truststoreFile;
private static File keystoreFile;
private static Config createdConfig;

@BeforeAll
public static void beforeAll() throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException {
Expand All @@ -60,14 +56,6 @@ public static void beforeAll() throws IOException, KeyStoreException, Certificat
keystore.store(new FileOutputStream(keystoreFile), KEYSTORE_PASSWORD.toCharArray());
}

@AfterEach
public void afterEach() {
if (createdConfig != null) {
ConfigProviderResolver.instance().releaseConfig(createdConfig);
createdConfig = null;
}
}

@Test
public void testClientSpecificConfigs() {
// given
Expand Down
Loading
Loading