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.

Sign up for GitHub

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

Upgrade vaadin and importmap and add caching #35712

Merged
merged 1 commit into from
Sep 5, 2023
Merged

Upgrade vaadin and importmap and add caching #35712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions bom/application/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,8 @@
<pulsar-client.version>3.0.0</pulsar-client.version>
<async-http-client.version>2.12.3</async-http-client.version>
<!-- Dev UI -->
<importmap.version>1.0.9</importmap.version>
<vaadin.version>24.1.4</vaadin.version>
<importmap.version>1.0.10</importmap.version>
<vaadin.version>24.1.6</vaadin.version>
<lit.version>2.8.0</lit.version>
<lit-element.version>3.3.3</lit-element.version>
<lit-html.version>2.8.0</lit-html.version>
Expand Down
41 changes: 33 additions & 8 deletions extensions/vertx-http/runtime/src/main/java/io/quarkus/devui/runtime/MvnpmHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@
import java.io.UncheckedIOException;
import java.net.URL;
import java.net.URLClassLoader;
import java.net.URLConnection;
import java.time.Instant;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
import java.time.temporal.TemporalAccessor;
import java.util.Locale;
import java.util.Set;

import io.vertx.core.Handler;
Expand All @@ -22,7 +29,7 @@ public class MvnpmHandler implements Handler<RoutingContext> {

public MvnpmHandler(String root, Set<URL> mvnpmJars) {
this.root = root;
this.mvnpmLoader = new URLClassLoader(mvnpmJars.toArray(new URL[] {}));
this.mvnpmLoader = new URLClassLoader(mvnpmJars.toArray(URL[]::new));
}

@Override
Expand All @@ -37,20 +44,38 @@ public void handle(RoutingContext event) {
}

try {
InputStream is = mvnpmLoader.getResourceAsStream(BASE_DIR + fullPath);
if (is != null) {
byte[] contents = is.readAllBytes();
event.response()
.putHeader(HttpHeaders.CONTENT_TYPE, getContentType(fileName))
.end(Buffer.buffer(contents));
return;
URL url = mvnpmLoader.getResource(BASE_DIR + fullPath);
URLConnection openConnection = url.openConnection();
long lastModified = openConnection.getLastModified();
try (InputStream is = openConnection.getInputStream()) {
if (is != null) {
byte[] contents = is.readAllBytes();
event.response()
.putHeader(HttpHeaders.CONTENT_TYPE, getContentType(fileName))
.putHeader(HttpHeaders.CACHE_CONTROL, "public, immutable, max-age=31536000")
.putHeader(HttpHeaders.LAST_MODIFIED, formatDate(lastModified))
.putHeader("date", formatDate(LocalDateTime.now()))
.end(Buffer.buffer(contents));
return;
}
}
} catch (IOException ex) {
throw new UncheckedIOException(ex);
}
event.next();
}

private String formatDate(long m) {
Instant i = Instant.ofEpochMilli(m);
return formatDate(i);
}

private String formatDate(TemporalAccessor t) {
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("EEE, dd MMM yyyy HH:mm:ss z", Locale.ENGLISH)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that a bit expensive?
Isn't that the API that was made thread-safe?

@geoand do you know?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that the API that was made thread-safe

Correct.

Isn't that a bit expensive?

Yeah, it's not very performant AFAIK. Netty has DateFormatter but I don't know how it compares and if it has been optimized. @franz1981 do you know?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's an hot path probably worth using a whole different pattern...
The easier thing is to save in a static final field the formatter, which is immutable and thread safe (please check the doc yourself, I am still on PTO so I have limited GitHub access..).

The best would be to have a volatile field with the formatted string, updated using a timer (or some scheduled task) with second granularity. It's less costly and in the hot path the best solution overall.

Copy link
Contributor

@franz1981 franz1981 Sep 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As usual, measure, not guess; check if the date formatting is a costly hot path and you have few options to reduce its overhead...
@geoand I didn't checked Netty yet: will do with my beloved desktop at hand 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is Dev mode only, and also Dev UI only, so for now I think we can merge.

.withZone(ZoneId.of("GMT"));
return formatter.format(t);
}

private String getContentType(String filename) {
String f = filename.toLowerCase();
if (f.endsWith(DOT_JS)) {
Expand Down