Skip to content

Commit 5f125fd

Browse files
jknackjknack
authored
Merge pull request #3944 from jooby-project/3943
joobyRun: optimize hot-reload latency with debouncer and smart compil…
2 parents c5651e9 + 0a53f05 commit 5f125fd

3 files changed

Lines changed: 128 additions & 96 deletions

File tree

‎modules/jooby-run/src/main/java/io/jooby/internal/run/JoobyModuleFinder.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,18 @@
2222
import java.util.LinkedHashSet;
2323
import java.util.Set;
2424
import java.util.jar.JarFile;
25+
import java.util.regex.Pattern;
2526

2627
import org.jboss.modules.*;
2728
import org.jboss.modules.filter.PathFilters;
2829

2930
import io.jooby.run.JoobyRun;
3031

3132
public abstract class JoobyModuleFinder implements ModuleFinder {
33+
// Matches logback, log4j, and application config files with optional environment suffixes
34+
private static final Pattern EXCLUDE_CONFIG =
35+
Pattern.compile(
36+
"^(logback.*\\.xml|log4j.*\\.(xml|properties|yaml|yml|json)|application.*\\.(conf|properties|yaml|yml|json))$");
3237
protected static final String JARS = "jars";
3338
protected static final String RESOURCES = "resources";
3439
protected final Set<Path> classes;
@@ -100,14 +105,7 @@ private ModuleSpec.Builder newModule(String name, Set<Path> resources) {
100105
if (main.equals(name)) {
101106
resourceLoader =
102107
createFilteredResourceLoader(
103-
not(
104-
it ->
105-
// remove duplicated log configuration
106-
(it.startsWith("logback") || it.startsWith("log4j"))
107-
&& it.endsWith(".xml")
108-
// remove duplicated configuration
109-
|| (it.startsWith("application") && it.endsWith(".conf"))),
110-
resourceLoader);
108+
not(it -> EXCLUDE_CONFIG.matcher(it).matches()), resourceLoader);
111109
}
112110
builder.addResourceRoot(ResourceLoaderSpec.createResourceLoaderSpec(resourceLoader));
113111
} else {

‎modules/jooby-run/src/main/java/io/jooby/run/JoobyRun.java‎

Lines changed: 99 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@
1414
import java.nio.file.Path;
1515
import java.time.Clock;
1616
import java.util.*;
17+
import java.util.concurrent.CompletableFuture;
1718
import java.util.concurrent.ConcurrentLinkedQueue;
1819
import java.util.concurrent.Executors;
1920
import java.util.concurrent.ScheduledExecutorService;
21+
import java.util.concurrent.ScheduledFuture;
2022
import java.util.concurrent.TimeUnit;
21-
import java.util.concurrent.atomic.AtomicInteger;
23+
import java.util.concurrent.atomic.AtomicReference;
2224
import java.util.function.BiConsumer;
2325
import java.util.function.Supplier;
2426
import java.util.stream.Collectors;
@@ -51,19 +53,24 @@ public class JoobyRun {
5153
private record Event(Path path, long time, Supplier<Boolean> compileTask) {}
5254

5355
private static class AppModule {
56+
57+
private enum State {
58+
CLOSED,
59+
UNLOADING,
60+
UNLOADED,
61+
STARTING,
62+
RESTART,
63+
RUNNING,
64+
FAILED
65+
}
66+
5467
private final Logger logger;
5568
private final JoobyModuleLoader loader;
5669
private final JoobyRunOptions conf;
5770
private Module module;
58-
private ClassLoader contextClassLoader;
71+
private final ClassLoader contextClassLoader;
5972
private int counter;
60-
private final AtomicInteger state = new AtomicInteger(CLOSED);
61-
private static final int CLOSED = 1 << 0;
62-
private static final int UNLOADING = 1 << 1;
63-
private static final int UNLOADED = 1 << 2;
64-
private static final int STARTING = 1 << 3;
65-
private static final int RESTART = 1 << 4;
66-
private static final int RUNNING = 1 << 5;
73+
private final AtomicReference<State> state = new AtomicReference<>(State.CLOSED);
6774

6875
AppModule(
6976
Logger logger,
@@ -77,10 +84,14 @@ private static class AppModule {
7784
}
7885

7986
public Exception start() {
80-
if (!(state.compareAndSet(CLOSED, STARTING) || state.compareAndSet(UNLOADED, STARTING))) {
87+
if (!(state.compareAndSet(State.CLOSED, State.STARTING)
88+
|| state.compareAndSet(State.UNLOADED, State.STARTING))) {
8189
debugState("Jooby already starting.");
8290
return null;
8391
}
92+
93+
boolean success = false;
94+
8495
try {
8596
module = loader.loadModule(conf.getProjectName());
8697
ModuleClassLoader classLoader = module.getClassLoader();
@@ -103,6 +114,7 @@ public Exception start() {
103114
args.add("server.port=" + port);
104115
}
105116
module.run(conf.getMainClass(), args.toArray(new String[0]));
117+
success = true; // Execution reached the end without throwing
106118
} catch (ClassNotFoundException x) {
107119
String message = x.getMessage();
108120
if (message.trim().startsWith(conf.getMainClass())) {
@@ -119,8 +131,13 @@ public Exception start() {
119131
} catch (Throwable x) {
120132
printErr(x);
121133
} finally {
122-
if (state.compareAndSet(STARTING, RUNNING)) {
123-
debugState("Jooby is now");
134+
if (success) {
135+
if (state.compareAndSet(State.STARTING, State.RUNNING)) {
136+
debugState("Jooby is now");
137+
}
138+
} else {
139+
state.set(State.FAILED);
140+
debugState("Jooby start failed");
124141
}
125142
Thread.currentThread().setContextClassLoader(contextClassLoader);
126143
}
@@ -159,22 +176,28 @@ private boolean isFatal(Throwable cause) {
159176
}
160177

161178
public boolean isStarting() {
162-
long s = state.longValue();
163-
return s > CLOSED && s < RUNNING;
179+
State s = state.get();
180+
return s == State.UNLOADING
181+
|| s == State.UNLOADED
182+
|| s == State.STARTING
183+
|| s == State.RESTART;
164184
}
165185

166186
public void restart(boolean unload) {
167-
if (state.compareAndSet(RUNNING, RESTART)) {
168-
// Shutdown
187+
// Allow restart if it's currently running OR if it previously failed to start
188+
if (state.compareAndSet(State.RUNNING, State.RESTART)
189+
|| state.compareAndSet(State.FAILED, State.RESTART)) {
190+
// Shutdown old state
169191
closeServer();
170192
if (unload) {
171-
// unload only when a class has changed
172193
unloadModule();
173194
}
174-
// Start
195+
196+
// Start new state
175197
start();
176-
// Run gc
177-
System.gc();
198+
199+
// Run gc asynchronously to clear discarded classloaders without blocking the thread
200+
CompletableFuture.runAsync(System::gc);
178201
} else {
179202
debugState("Already restarting.");
180203
}
@@ -195,7 +218,7 @@ private Throwable withoutReflection(Throwable cause) {
195218
}
196219

197220
private void unloadModule() {
198-
if (!state.compareAndSet(CLOSED, UNLOADING)) {
221+
if (!state.compareAndSet(State.CLOSED, State.UNLOADING)) {
199222
debugState("Cannot unload as server isn't closed.");
200223
return;
201224
}
@@ -206,49 +229,28 @@ private void unloadModule() {
206229
} catch (Exception x) {
207230
logger.debug("unload module resulted in exception", x);
208231
} finally {
209-
state.compareAndSet(UNLOADING, UNLOADED);
232+
state.compareAndSet(State.UNLOADING, State.UNLOADED);
210233
module = null;
211234
}
212235
}
213236

214237
private void closeServer() {
215238
try {
216239
debugState("Closing server.");
217-
Class<?> ref = module.getClassLoader().loadClass(SERVER_REF);
218-
ref.getDeclaredMethod(SERVER_REF_STOP).invoke(null);
240+
if (module != null) {
241+
Class<?> ref = module.getClassLoader().loadClass(SERVER_REF);
242+
ref.getDeclaredMethod(SERVER_REF_STOP).invoke(null);
243+
}
219244
} catch (Exception x) {
220245
logger.error("Application shutdown resulted in exception", withoutReflection(x));
221246
} finally {
222-
state.set(CLOSED);
247+
state.set(State.CLOSED);
223248
}
224249
}
225250

226251
private void debugState(String message) {
227252
if (logger.isDebugEnabled()) {
228-
String name;
229-
switch (state.get()) {
230-
case CLOSED:
231-
name = "CLOSED";
232-
break;
233-
case UNLOADING:
234-
name = "UNLOADING";
235-
break;
236-
case UNLOADED:
237-
name = "UNLOADED";
238-
break;
239-
case STARTING:
240-
name = "STARTING";
241-
break;
242-
case RESTART:
243-
name = "RESTART";
244-
break;
245-
case RUNNING:
246-
name = "RUNNING";
247-
break;
248-
default:
249-
throw new IllegalStateException("BUG");
250-
}
251-
logger.debug("{} state: {}", message, name);
253+
logger.debug("{} state: {}", message, state.get().name());
252254
}
253255
}
254256
}
@@ -275,13 +277,15 @@ private void debugState(String message) {
275277

276278
private final ConcurrentLinkedQueue<Event> queue = new ConcurrentLinkedQueue<>();
277279

280+
// Executor and task tracking for the sliding-window debouncer
281+
private ScheduledExecutorService se;
282+
private volatile ScheduledFuture<?> scheduledRestart;
283+
278284
/*
279285
* How long we wait after the last change before restart
280286
*/
281287
private final long waitTimeBeforeRestartMillis;
282288

283-
private final long initialDelayBeforeFirstRestartMillis;
284-
285289
/**
286290
* Creates a new instances with the given options.
287291
*
@@ -291,8 +295,6 @@ public JoobyRun(JoobyRunOptions options) {
291295
this.options = options;
292296
clock = Clock.systemUTC(); // Possibly change for unit test
293297
waitTimeBeforeRestartMillis = options.getWaitTimeBeforeRestart();
294-
// this might not need to be configurable
295-
initialDelayBeforeFirstRestartMillis = JoobyRunOptions.INITIAL_DELAY_BEFORE_FIRST_RESTART;
296298
}
297299

298300
/**
@@ -389,19 +391,16 @@ public void start() throws Throwable {
389391
new JoobyModuleLoader(finder),
390392
Thread.currentThread().getContextClassLoader(),
391393
options);
392-
ScheduledExecutorService se;
394+
393395
Exception error = module.start();
394396
if (error == null) {
395397
se = Executors.newScheduledThreadPool(1);
396-
se.scheduleAtFixedRate(
397-
this::actualRestart,
398-
initialDelayBeforeFirstRestartMillis,
399-
waitTimeBeforeRestartMillis,
400-
TimeUnit.MILLISECONDS);
401398
try {
402399
watcher.watch();
403400
} finally {
404-
se.shutdownNow();
401+
if (se != null) {
402+
se.shutdownNow();
403+
}
405404
}
406405
} else {
407406
// exit
@@ -419,35 +418,50 @@ public void restart(Path path) {
419418
}
420419

421420
public void restart(Path path, Supplier<Boolean> compileTask) {
421+
// 1. Queue the event
422422
queue.offer(new Event(path, clock.millis(), compileTask));
423-
}
424423

425-
private synchronized void actualRestart() {
426-
if (module.isStarting()) {
427-
return; // We don't empty the queue. This is the case a change was made while starting.
424+
// 2. Cancel the pending restart if it hasn't executed yet (Sliding Window)
425+
if (scheduledRestart != null && !scheduledRestart.isDone()) {
426+
scheduledRestart.cancel(false);
428427
}
429-
long t = clock.millis();
430-
Event e = queue.peek();
431-
if (e == null) {
432-
return; // queue was empty
428+
429+
// 3. Schedule a new restart after the wait period has elapsed since the LAST file change
430+
if (se != null && !se.isShutdown()) {
431+
scheduledRestart =
432+
se.schedule(
433+
this::processQueueAndRestart, waitTimeBeforeRestartMillis, TimeUnit.MILLISECONDS);
434+
}
435+
}
436+
437+
private void processQueueAndRestart() {
438+
if (module == null || module.isStarting()) {
439+
return;
433440
}
434-
var unload = false;
441+
442+
Event e;
443+
boolean unload = false;
435444
Supplier<Boolean> compileTask = null;
436-
for (; e != null && (t - e.time) > waitTimeBeforeRestartMillis; e = queue.peek()) {
437-
// unload on source code changes (.java, .kt) or binary changes (.class)
445+
boolean hasEvents = false;
446+
447+
// Drain the entire queue as a single batch
448+
while ((e = queue.poll()) != null) {
449+
hasEvents = true;
438450
unload = unload || options.isCompileExtension(e.path) || options.isClass(e.path);
439451
compileTask = Optional.ofNullable(compileTask).orElse(e.compileTask);
440-
queue.poll();
441452
}
442-
// e will be null if the queue is empty which means all events were old enough
443-
if (e == null) {
444-
var restart = true;
445-
if (compileTask != null) {
446-
restart = compileTask.get();
447-
}
448-
if (restart) {
449-
module.restart(unload);
450-
}
453+
454+
if (!hasEvents) {
455+
return;
456+
}
457+
458+
boolean restart = true;
459+
if (compileTask != null) {
460+
restart = compileTask.get();
461+
}
462+
463+
if (restart) {
464+
module.restart(unload);
451465
}
452466
}
453467

@@ -458,6 +472,10 @@ public void shutdown() {
458472
module = null;
459473
}
460474

475+
if (se != null) {
476+
se.shutdownNow();
477+
}
478+
461479
if (watcher != null) {
462480
try {
463481
watcher.close();

0 commit comments

Comments
 (0)