From ce0e02a167b166399f1ed860d9415aff050c71ca Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Wed, 27 Sep 2023 16:24:16 +0800 Subject: [PATCH] perf: optimizing request plugin bundle resources (#4639) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind improvement /area core /milestone 2.10.x #### What this PR does / why we need it: 优化插件捆绑资源的查询性能 在插件 js bundle 1.2M 大小的情况下： 优化前： image 优化后： image how to test it? 在插件生产模式下使用此 PR 测试，比对与不使用此 PR 的 API 速度，且检查 js bundle 和 css bundle 的资源是否正确加载。 #### Does this PR introduce a user-facing change? ```release-note 优化插件捆绑资源(JSBundle)的查询性能以提高页面加载速度 ``` --- .../extension/endpoint/PluginEndpoint.java | 189 ++++++++++++++++-- .../core/extension/service/PluginService.java | 5 +- .../service/impl/PluginServiceImpl.java | 92 ++++----- .../endpoint/PluginEndpointTest.java | 100 ++++++++- 4 files changed, 313 insertions(+), 73 deletions(-) diff --git a/application/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java b/application/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java index 995f8d33c..0a3ca14c3 100644 --- a/application/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java +++ b/application/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java @@ -19,11 +19,13 @@ import static run.halo.app.infra.utils.FileUtils.deleteFileSilently; import io.swagger.v3.oas.annotations.enums.ParameterIn; import io.swagger.v3.oas.annotations.media.ArraySchema; import io.swagger.v3.oas.annotations.media.Schema; +import java.io.IOException; import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.time.Duration; +import java.time.Instant; import java.util.ArrayList; import java.util.Comparator; import java.util.List; @@ -31,13 +33,21 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Function; import java.util.function.Predicate; +import java.util.function.Supplier; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.reactivestreams.Publisher; import org.springdoc.webflux.core.fn.SpringdocRouteBuilder; +import org.springframework.beans.factory.DisposableBean; +import org.springframework.core.io.FileSystemResource; +import org.springframework.core.io.Resource; import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.domain.Sort; import org.springframework.http.CacheControl; @@ -45,14 +55,19 @@ import org.springframework.http.MediaType; import org.springframework.http.codec.multipart.FilePart; import org.springframework.http.codec.multipart.FormFieldPart; import org.springframework.http.codec.multipart.Part; +import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; +import org.springframework.util.Assert; +import org.springframework.util.FileSystemUtils; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; +import org.springframework.web.reactive.function.BodyInserters; import org.springframework.web.reactive.function.server.RouterFunction; import org.springframework.web.reactive.function.server.ServerRequest; import org.springframework.web.reactive.function.server.ServerResponse; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.ServerWebInputException; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; import reactor.core.scheduler.Schedulers; @@ -82,6 +97,8 @@ public class PluginEndpoint implements CustomEndpoint { private final Scheduler scheduler = Schedulers.boundedElastic(); + private final BufferedPluginBundleResource bufferedPluginBundleResource; + @Override public RouterFunction endpoint() { final var tag = "api.console.halo.run/v1alpha1/Plugin"; @@ -240,36 +257,59 @@ public class PluginEndpoint implements CustomEndpoint { private Mono fetchJsBundle(ServerRequest request) { Optional versionOption = request.queryParam("v"); - if (versionOption.isEmpty()) { - return pluginService.generateJsBundleVersion() + return versionOption.map(s -> + Mono.defer(() -> bufferedPluginBundleResource + .getJsBundle(s, pluginService::uglifyJsBundle) + ).flatMap(fsRes -> { + var bodyBuilder = ServerResponse.ok() + .cacheControl(MAX_CACHE_CONTROL) + .contentType(MediaType.valueOf("text/javascript")); + try { + Instant lastModified = Instant.ofEpochMilli(fsRes.lastModified()); + return request.checkNotModified(lastModified) + .switchIfEmpty(Mono.defer(() -> + bodyBuilder.lastModified(lastModified) + .body(BodyInserters.fromResource(fsRes))) + ); + } catch (IOException e) { + return Mono.error(e); + } + }) + ) + .orElseGet(() -> pluginService.generateJsBundleVersion() .flatMap(v -> ServerResponse .temporaryRedirect(buildJsBundleUri("js", v)) .build() - ); - } - return pluginService.uglifyJsBundle() - .defaultIfEmpty("") - .flatMap(bundle -> ServerResponse.ok() - .cacheControl(MAX_CACHE_CONTROL) - .contentType(MediaType.valueOf("text/javascript")) - .bodyValue(bundle) + ) ); } private Mono fetchCssBundle(ServerRequest request) { Optional versionOption = request.queryParam("v"); - if (versionOption.isEmpty()) { - return pluginService.generateJsBundleVersion() + return versionOption.map(s -> + Mono.defer(() -> bufferedPluginBundleResource.getCssBundle(s, + pluginService::uglifyCssBundle) + ).flatMap(fsRes -> { + var bodyBuilder = ServerResponse.ok() + .cacheControl(MAX_CACHE_CONTROL) + .contentType(MediaType.valueOf("text/css")); + try { + Instant lastModified = Instant.ofEpochMilli(fsRes.lastModified()); + return request.checkNotModified(lastModified) + .switchIfEmpty(Mono.defer(() -> + bodyBuilder.lastModified(lastModified) + .body(BodyInserters.fromResource(fsRes))) + ); + } catch (IOException e) { + return Mono.error(e); + } + }) + ) + .orElseGet(() -> pluginService.generateJsBundleVersion() .flatMap(v -> ServerResponse .temporaryRedirect(buildJsBundleUri("css", v)) .build() - ); - } - return pluginService.uglifyCssBundle() - .flatMap(bundle -> ServerResponse.ok() - .cacheControl(MAX_CACHE_CONTROL) - .contentType(MediaType.valueOf("text/css")) - .bodyValue(bundle) + ) ); } @@ -353,7 +393,7 @@ public class PluginEndpoint implements CustomEndpoint { if (!configMapName.equals(configMapNameToUpdate)) { throw new ServerWebInputException( "The name from the request body does not match the plugin " - + "configMapName name."); + + "configMapName name."); } }) .flatMap(configMapToUpdate -> client.fetch(ConfigMap.class, configMapName) @@ -482,7 +522,7 @@ public class PluginEndpoint implements CustomEndpoint { @ArraySchema(uniqueItems = true, arraySchema = @Schema(name = "sort", description = "Sort property and direction of the list result. Supported fields: " - + "creationTimestamp"), + + "creationTimestamp"), schema = @Schema(description = "like field,asc or field,desc", implementation = String.class, example = "creationTimestamp,desc")) @@ -633,4 +673,111 @@ public class PluginEndpoint implements CustomEndpoint { .subscribeOn(this.scheduler); } + @Component + static class BufferedPluginBundleResource implements DisposableBean { + + private final AtomicReference jsBundle = new AtomicReference<>(); + private final AtomicReference cssBundle = new AtomicReference<>(); + + private final ReadWriteLock jsLock = new ReentrantReadWriteLock(); + private final ReadWriteLock cssLock = new ReentrantReadWriteLock(); + + private Path tempDir; + + public Mono getJsBundle(String version, + Supplier> jsSupplier) { + var fileName = tempFileName(version, ".js"); + return Mono.defer(() -> { + jsLock.readLock().lock(); + try { + var jsBundleResource = jsBundle.get(); + if (getResourceIfNotChange(fileName, jsBundleResource) != null) { + return Mono.just(jsBundleResource); + } + } finally { + jsLock.readLock().unlock(); + } + + jsLock.writeLock().lock(); + try { + var oldJsBundle = jsBundle.get(); + return writeBundle(fileName, jsSupplier) + .doOnNext(newRes -> jsBundle.compareAndSet(oldJsBundle, newRes)); + } finally { + jsLock.writeLock().unlock(); + } + }).subscribeOn(Schedulers.boundedElastic()); + } + + public Mono getCssBundle(String version, + Supplier> cssSupplier) { + var fileName = tempFileName(version, ".css"); + return Mono.defer(() -> { + try { + cssLock.readLock().lock(); + var cssBundleResource = cssBundle.get(); + if (getResourceIfNotChange(fileName, cssBundleResource) != null) { + return Mono.just(cssBundleResource); + } + } finally { + cssLock.readLock().unlock(); + } + + cssLock.writeLock().lock(); + try { + var oldCssBundle = cssBundle.get(); + return writeBundle(fileName, cssSupplier) + .doOnNext(newRes -> cssBundle.compareAndSet(oldCssBundle, newRes)); + } finally { + cssLock.writeLock().unlock(); + } + }).subscribeOn(Schedulers.boundedElastic()); + } + + @Nullable + private Resource getResourceIfNotChange(String fileName, Resource resource) { + if (resource != null && resource.exists() && fileName.equals(resource.getFilename())) { + return resource; + } + return null; + } + + private Mono writeBundle(String fileName, + Supplier> dataSupplier) { + return Mono.defer( + () -> { + var filePath = createTempFileToStore(fileName); + return DataBufferUtils.write(dataSupplier.get(), filePath) + .then(Mono.fromSupplier(() -> new FileSystemResource(filePath))); + }); + } + + private Path createTempFileToStore(String fileName) { + try { + if (tempDir == null || !Files.exists(tempDir)) { + this.tempDir = Files.createTempDirectory("halo-plugin-bundle"); + } + var path = tempDir.resolve(fileName); + Files.deleteIfExists(path); + return Files.createFile(path); + } catch (IOException e) { + throw new ServerWebInputException("Failed to create temp file.", null, e); + } + } + + private String tempFileName(String v, String suffix) { + Assert.notNull(v, "Version must not be null"); + Assert.notNull(suffix, "Suffix must not be null"); + return v + suffix; + } + + @Override + public void destroy() throws Exception { + if (tempDir != null && Files.exists(tempDir)) { + FileSystemUtils.deleteRecursively(tempDir); + } + this.jsBundle.set(null); + this.cssBundle.set(null); + } + } } diff --git a/application/src/main/java/run/halo/app/core/extension/service/PluginService.java b/application/src/main/java/run/halo/app/core/extension/service/PluginService.java index bcf404dbe..27e4f43ff 100644 --- a/application/src/main/java/run/halo/app/core/extension/service/PluginService.java +++ b/application/src/main/java/run/halo/app/core/extension/service/PluginService.java @@ -1,6 +1,7 @@ package run.halo.app.core.extension.service; import java.nio.file.Path; +import org.springframework.core.io.buffer.DataBuffer; import org.springframework.web.server.ServerWebInputException; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -46,14 +47,14 @@ public interface PluginService { * * @return uglified js bundle */ - Mono uglifyJsBundle(); + Flux uglifyJsBundle(); /** * Uglify css bundle from all enabled plugins to a single css bundle string. * * @return uglified css bundle */ - Mono uglifyCssBundle(); + Flux uglifyCssBundle(); /** *

Generate js bundle version for cache control.

diff --git a/application/src/main/java/run/halo/app/core/extension/service/impl/PluginServiceImpl.java b/application/src/main/java/run/halo/app/core/extension/service/impl/PluginServiceImpl.java index e5c621c10..ab029aa69 100644 --- a/application/src/main/java/run/halo/app/core/extension/service/impl/PluginServiceImpl.java +++ b/application/src/main/java/run/halo/app/core/extension/service/impl/PluginServiceImpl.java @@ -9,7 +9,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.ArrayList; import java.util.Comparator; import java.util.List; import java.util.Map; @@ -21,9 +20,13 @@ import org.apache.commons.lang3.Validate; import org.pf4j.PluginWrapper; import org.pf4j.RuntimeMode; import org.springframework.core.io.Resource; +import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DataBufferUtils; +import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.core.io.support.PathMatchingResourcePatternResolver; import org.springframework.stereotype.Component; import org.springframework.util.Assert; +import org.springframework.util.StreamUtils; import org.springframework.web.server.ServerWebInputException; import reactor.core.Exceptions; import reactor.core.publisher.Flux; @@ -133,55 +136,52 @@ public class PluginServiceImpl implements PluginService { } @Override - public Mono uglifyJsBundle() { - return Mono.fromSupplier(() -> { - StringBuilder jsBundle = new StringBuilder(); - List pluginNames = new ArrayList<>(); - for (PluginWrapper pluginWrapper : pluginManager.getStartedPlugins()) { - String pluginName = pluginWrapper.getPluginId(); - pluginNames.add(pluginName); - Resource jsBundleResource = - BundleResourceUtils.getJsBundleResource(pluginManager, pluginName, - BundleResourceUtils.JS_BUNDLE); - if (jsBundleResource != null) { - try { - jsBundle.append( - jsBundleResource.getContentAsString(StandardCharsets.UTF_8)); - jsBundle.append("\n"); - } catch (IOException e) { - log.error("Failed to read js bundle of plugin [{}]", pluginName, e); - } + public Flux uglifyJsBundle() { + var startedPlugins = List.copyOf(pluginManager.getStartedPlugins()); + String plugins = """ + this.enabledPluginNames = [%s]; + """.formatted(startedPlugins.stream() + .map(PluginWrapper::getPluginId) + .collect(Collectors.joining("','", "'", "'"))); + return Flux.fromIterable(startedPlugins) + .mapNotNull(pluginWrapper -> { + var pluginName = pluginWrapper.getPluginId(); + return BundleResourceUtils.getJsBundleResource(pluginManager, pluginName, + BundleResourceUtils.JS_BUNDLE); + }) + .flatMap(resource -> { + try { + // Specifying bufferSize as resource content length is + // to append line breaks at the end of each plugin + return DataBufferUtils.read(resource, DefaultDataBufferFactory.sharedInstance, + (int) resource.contentLength()) + .doOnNext(dataBuffer -> { + // add a new line after each plugin bundle to avoid syntax error + dataBuffer.write("\n".getBytes(StandardCharsets.UTF_8)); + }); + } catch (IOException e) { + log.error("Failed to read plugin bundle resource", e); + return Flux.empty(); } - } - - String plugins = """ - this.enabledPluginNames = [%s]; - """.formatted(pluginNames.stream() - .collect(Collectors.joining("','", "'", "'"))); - return jsBundle + plugins; - }); + }) + .concatWith(Flux.defer(() -> { + var dataBuffer = DefaultDataBufferFactory.sharedInstance + .wrap(plugins.getBytes(StandardCharsets.UTF_8)); + return Flux.just(dataBuffer); + })); } @Override - public Mono uglifyCssBundle() { - return Mono.fromSupplier(() -> { - StringBuilder cssBundle = new StringBuilder(); - for (PluginWrapper pluginWrapper : pluginManager.getStartedPlugins()) { + public Flux uglifyCssBundle() { + return Flux.fromIterable(pluginManager.getStartedPlugins()) + .mapNotNull(pluginWrapper -> { String pluginName = pluginWrapper.getPluginId(); - Resource cssBundleResource = - BundleResourceUtils.getJsBundleResource(pluginManager, pluginName, - BundleResourceUtils.CSS_BUNDLE); - if (cssBundleResource != null) { - try { - cssBundle.append( - cssBundleResource.getContentAsString(StandardCharsets.UTF_8)); - } catch (IOException e) { - log.error("Failed to read css bundle of plugin [{}]", pluginName, e); - } - } - } - return cssBundle.toString(); - }); + return BundleResourceUtils.getJsBundleResource(pluginManager, pluginName, + BundleResourceUtils.CSS_BUNDLE); + }) + .flatMap(resource -> DataBufferUtils.read(resource, + DefaultDataBufferFactory.sharedInstance, StreamUtils.BUFFER_SIZE) + ); } @Override @@ -259,7 +259,7 @@ public class PluginServiceImpl implements PluginService { if (!VersionUtils.satisfiesRequires(systemVersion, requires)) { throw new UnsatisfiedAttributeValueException(String.format( "Plugin requires a minimum system version of [%s], but the current version is " - + "[%s].", + + "[%s].", requires, systemVersion), "problemDetail.plugin.version.unsatisfied.requires", new String[] {requires, systemVersion}); diff --git a/application/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java b/application/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java index c11e2c778..84305941d 100644 --- a/application/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java +++ b/application/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java @@ -1,6 +1,7 @@ package run.halo.app.core.extension.endpoint; import static java.util.Objects.requireNonNull; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.argThat; @@ -16,6 +17,7 @@ import static org.springframework.web.reactive.function.BodyInserters.fromMultip import com.github.zafarkhaja.semver.Version; import java.io.IOException; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -33,11 +35,15 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.core.io.FileSystemResource; +import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.MediaType; import org.springframework.http.client.MultipartBodyBuilder; import org.springframework.test.web.reactive.server.WebTestClient; import org.springframework.web.server.ServerWebInputException; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; import run.halo.app.core.extension.Plugin; import run.halo.app.core.extension.Setting; import run.halo.app.core.extension.service.PluginService; @@ -130,8 +136,8 @@ class PluginEndpointTest { verify(client).list(same(Plugin.class), argThat( predicate -> predicate.test(expectPlugin) - && !predicate.test(unexpectedPlugin1) - && !predicate.test(unexpectedPlugin2)), + && !predicate.test(unexpectedPlugin1) + && !predicate.test(unexpectedPlugin2)), any(), anyInt(), anyInt()); } @@ -158,8 +164,8 @@ class PluginEndpointTest { verify(client).list(same(Plugin.class), argThat( predicate -> predicate.test(expectPlugin) - && !predicate.test(unexpectedPlugin1) - && !predicate.test(unexpectedPlugin2)), + && !predicate.test(unexpectedPlugin1) + && !predicate.test(unexpectedPlugin2)), any(), anyInt(), anyInt()); } @@ -382,4 +388,90 @@ class PluginEndpointTest { plugin.setSpec(spec); return plugin; } + + @Nested + class BufferedPluginBundleResourceTest { + private final PluginEndpoint.BufferedPluginBundleResource bufferedPluginBundleResource = + new PluginEndpoint.BufferedPluginBundleResource(); + + private static Flux getDataBufferFlux(String x) { + var buffer = DefaultDataBufferFactory.sharedInstance + .wrap(x.getBytes(StandardCharsets.UTF_8)); + return Flux.just(buffer); + } + + @Test + void writeAndGetJsResourceTest() { + bufferedPluginBundleResource.getJsBundle("1", + () -> getDataBufferFlux("first line\nnext line")) + .as(StepVerifier::create) + .consumeNextWith(resource -> { + try { + String content = resource.getContentAsString(StandardCharsets.UTF_8); + assertThat(content).isEqualTo("first line\nnext line"); + } catch (IOException e) { + throw new RuntimeException(e); + } + }) + .verifyComplete(); + + // version is matched, should return cached content + bufferedPluginBundleResource.getJsBundle("1", + () -> getDataBufferFlux("first line\nnext line-1")) + .as(StepVerifier::create) + .consumeNextWith(resource -> { + try { + String content = resource.getContentAsString(StandardCharsets.UTF_8); + assertThat(content).isEqualTo("first line\nnext line"); + } catch (IOException e) { + throw new RuntimeException(e); + } + }) + .verifyComplete(); + + // new version should return new content + bufferedPluginBundleResource.getJsBundle("2", + () -> getDataBufferFlux("first line\nnext line-2")) + .as(StepVerifier::create) + .consumeNextWith(resource -> { + try { + String content = resource.getContentAsString(StandardCharsets.UTF_8); + assertThat(content).isEqualTo("first line\nnext line-2"); + } catch (IOException e) { + throw new RuntimeException(e); + } + }) + .verifyComplete(); + } + + @Test + void writeAndGetCssResourceTest() { + bufferedPluginBundleResource.getCssBundle("1", + () -> getDataBufferFlux("first line\nnext line")) + .as(StepVerifier::create) + .consumeNextWith(resource -> { + try { + String content = resource.getContentAsString(StandardCharsets.UTF_8); + assertThat(content).isEqualTo("first line\nnext line"); + } catch (IOException e) { + throw new RuntimeException(e); + } + }) + .verifyComplete(); + + // version is matched, should return cached content + bufferedPluginBundleResource.getCssBundle("1", + () -> getDataBufferFlux("first line\nnext line-1")) + .as(StepVerifier::create) + .consumeNextWith(resource -> { + try { + String content = resource.getContentAsString(StandardCharsets.UTF_8); + assertThat(content).isEqualTo("first line\nnext line"); + } catch (IOException e) { + throw new RuntimeException(e); + } + }) + .verifyComplete(); + } + } }