From b03db7b3f9af1df7e92dacf15718508dd491876a Mon Sep 17 00:00:00 2001 From: DongHoon Lee Date: Sun, 20 Sep 2026 13:26:31 +0900 Subject: [PATCH] Check resource existence once when looking for deleted files ClassLoaderFilesResourcePatternResolver.isDeleted() calls resource.exists() and resource.getURI() inside the loop over the uploaded files, so both are repeated once per DELETED entry although neither depends on the entry. Every resource lookup made while the application context is being built goes through this method - about 1300 times per restart in the application I measured - so a session that has accumulated 100 deleted files performs around 130 000 file system lookups where 1300 are enough. This commit hoists both calls out of the loop: the resource is inspected at most once per call, on the first DELETED entry, and the URI comparison then uses the cached value. The method still returns on the first match and still reports a failing getURI() as an IllegalStateException, so behaviour is unchanged. The loop now walks the per-directory entry sets, which is what ClassLoaderFiles.addAll() and RestartServer already do, instead of the flattened view added in gh-46289. That view is wrapped in Collections.unmodifiableSet(), whose iterator is shared JDK code; in a running application its delegate calls are megamorphic and C2 stops inlining them, which made restarts with many accumulated files and no deletions slower than before gh-46289. isDeleted() returns a boolean, so the visiting order of the entries cannot change its result. Measured on a Spring Boot application driven through the remote restart path (restart request to ApplicationReadyEvent), 12 JVMs per variant and 20 restarts per JVM, medians: 100 source directories, 10 000 entries, 10 deleted: 278 ms -> 200 ms 100 source directories, 10 000 entries, 100 deleted: 891 ms -> 201 ms 100 source directories, 10 000 entries, 500 deleted: 3536 ms -> 200 ms 100 source directories, 50 000 entries, none deleted: 431 ms -> 341 ms 100 source directories, 10 000 entries, none deleted: 202 ms -> 192 ms no uploaded files: 150 ms -> 148 ms Signed-off-by: DongHoon Lee --- ...assLoaderFilesResourcePatternResolver.java | 33 ++++++++++---- ...aderFilesResourcePatternResolverTests.java | 44 +++++++++++++++++++ 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/module/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/ClassLoaderFilesResourcePatternResolver.java b/module/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/ClassLoaderFilesResourcePatternResolver.java index 7541b0947c45..27cc715ce835 100644 --- a/module/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/ClassLoaderFilesResourcePatternResolver.java +++ b/module/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/ClassLoaderFilesResourcePatternResolver.java @@ -33,6 +33,7 @@ import org.springframework.boot.devtools.restart.classloader.ClassLoaderFile.Kind; import org.springframework.boot.devtools.restart.classloader.ClassLoaderFileURLStreamHandler; import org.springframework.boot.devtools.restart.classloader.ClassLoaderFiles; +import org.springframework.boot.devtools.restart.classloader.ClassLoaderFiles.SourceDirectory; import org.springframework.context.ApplicationContext; import org.springframework.context.support.AbstractApplicationContext; import org.springframework.core.io.AbstractResource; @@ -58,6 +59,7 @@ * @author Andy Wilkinson * @author Phillip Webb * @author Stephane Nicoll + * @author DongHoon Lee */ final class ClassLoaderFilesResourcePatternResolver implements ResourcePatternResolver { @@ -146,22 +148,35 @@ private String trimLocationPattern(String pattern) { } private boolean isDeleted(Resource resource) { - for (Entry entry : this.classLoaderFiles.getFileEntries()) { - try { - String name = entry.getKey(); - ClassLoaderFile file = entry.getValue(); - if (file.getKind() == Kind.DELETED && resource.exists() - && resource.getURI().toString().endsWith(name)) { + String uri = null; + for (SourceDirectory sourceDirectory : this.classLoaderFiles.getSourceDirectories()) { + for (Entry entry : sourceDirectory.getFilesEntrySet()) { + if (entry.getValue().getKind() != Kind.DELETED) { + continue; + } + if (uri == null) { + if (!resource.exists()) { + return false; + } + uri = getUri(resource); + } + if (uri.endsWith(entry.getKey())) { return true; } } - catch (IOException ex) { - throw new IllegalStateException("Failed to retrieve URI from '" + resource + "'", ex); - } } return false; } + private String getUri(Resource resource) { + try { + return resource.getURI().toString(); + } + catch (IOException ex) { + throw new IllegalStateException("Failed to retrieve URI from '" + resource + "'", ex); + } + } + /** * A {@link Resource} that represents a {@link ClassLoaderFile} that has been * {@link Kind#DELETED deleted}. diff --git a/module/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/ClassLoaderFilesResourcePatternResolverTests.java b/module/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/ClassLoaderFilesResourcePatternResolverTests.java index 998b73c59bdc..68b9ba6f6a3a 100644 --- a/module/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/ClassLoaderFilesResourcePatternResolverTests.java +++ b/module/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/ClassLoaderFilesResourcePatternResolverTests.java @@ -18,6 +18,7 @@ import java.io.File; import java.io.IOException; +import java.net.URI; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -43,6 +44,7 @@ import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.then; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; /** * Tests for {@link ClassLoaderFilesResourcePatternResolver}. @@ -50,6 +52,7 @@ * @author Phillip Webb * @author Andy Wilkinson * @author Stephane Nicoll + * @author DongHoon Lee */ class ClassLoaderFilesResourcePatternResolverTests { @@ -90,6 +93,41 @@ void getResourceWhenDeletedShouldReturnDeletedResource(@TempDir File directory) assertThat(resource).isInstanceOf(DeletedClassLoaderFileResource.class); } + @Test + void getResourceWhenDeletedInAnotherSourceDirectoryShouldReturnDeletedResource(@TempDir File directory) + throws Exception { + File file = createFile(directory, "name.class"); + this.files.addFile("one", "other.class", new ClassLoaderFile(Kind.ADDED, new byte[0])); + this.files.addFile("two", "name.class", new ClassLoaderFile(Kind.DELETED, null)); + Resource resource = this.resolver.getResource("file:" + file.getAbsolutePath()); + assertThat(resource).isInstanceOf(DeletedClassLoaderFileResource.class); + } + + @Test + void getResourceWhenManyFilesAreDeletedShouldCheckResourceOnlyOnce() throws Exception { + Resource resource = mock(Resource.class); + given(resource.exists()).willReturn(true); + given(resource.getURI()).willReturn(URI.create("file:/app/classes/three.class")); + this.resolver = createResolverResolving("foo:some-file.txt", resource); + this.files.addFile("one", "one.class", new ClassLoaderFile(Kind.DELETED, null)); + this.files.addFile("one", "two.class", new ClassLoaderFile(Kind.DELETED, null)); + this.files.addFile("two", "three.class", new ClassLoaderFile(Kind.DELETED, null)); + assertThat(this.resolver.getResource("foo:some-file.txt")).isInstanceOf(DeletedClassLoaderFileResource.class); + then(resource).should().exists(); + then(resource).should().getURI(); + } + + @Test + void getResourceWhenNoFileIsDeletedShouldNotCheckResource() throws Exception { + Resource resource = mock(Resource.class); + this.resolver = createResolverResolving("foo:some-file.txt", resource); + this.files.addFile("one", "one.class", new ClassLoaderFile(Kind.ADDED, new byte[0])); + this.files.addFile("two", "two.class", new ClassLoaderFile(Kind.MODIFIED, new byte[0])); + assertThat(this.resolver.getResource("foo:some-file.txt")).isSameAs(resource); + then(resource).should(never()).exists(); + then(resource).should(never()).getURI(); + } + @Test void getResourcesShouldReturnResources(@TempDir File directory) throws Exception { File file = createFile(directory, "name.class"); @@ -173,6 +211,12 @@ void customProtocolResolverRegisteredAfterCreationIsUsedInWebApplication() { then(resolver).should().resolve(eq("foo:some-file.txt"), any(ResourceLoader.class)); } + private ClassLoaderFilesResourcePatternResolver createResolverResolving(String location, Resource resource) { + GenericApplicationContext context = new GenericApplicationContext(); + context.addProtocolResolver(mockProtocolResolver(location, resource)); + return new ClassLoaderFilesResourcePatternResolver(context, this.files); + } + private ProtocolResolver mockProtocolResolver(String path, Resource resource) { ProtocolResolver resolver = mock(ProtocolResolver.class); given(resolver.resolve(eq(path), any(ResourceLoader.class))).willReturn(resource);