gnodet-bot commented on code in PR #26653:
URL: https://github.com/apache/camel/pull/26653#discussion_r4059929509
##########
core/camel-support/src/main/java/org/apache/camel/support/FileWatcherResourceReloadStrategy.java:
##########
@@ -225,13 +231,62 @@ private void registerRecursive(final WatchService
watcher, final Path root, fina
Files.walkFileTree(root, new SimpleFileVisitor<>() {
@Override
public FileVisitResult preVisitDirectory(Path dir,
BasicFileAttributes attrs) throws IOException {
+ if (isCompileWorkDir(dir)) {
+ return FileVisitResult.SKIP_SUBTREE;
+ }
WatchKey key = registerPathToWatcher(modifier, dir, watcher);
folderKeys.put(key, dir);
return FileVisitResult.CONTINUE;
}
});
}
+ /**
+ * Registers a directory created while watching recursively, and its
subdirectories, and collects the files already
+ * in them as changes: a tree such as src/main/java/com/acme is usually
created with its first file in it, before
+ * the watcher can see the directory.
+ */
+ private void registerNewDirectory(Path dir, List<File> changed) {
+ try {
+ Files.walkFileTree(dir, new SimpleFileVisitor<>() {
+ @Override
+ public FileVisitResult preVisitDirectory(Path d,
BasicFileAttributes attrs) throws IOException {
+ if (isCompileWorkDir(d) || folderKeys.containsValue(d)) {
+ return FileVisitResult.SKIP_SUBTREE;
+ }
+ WatchKey k = registerPathToWatcher(watchModifier, d,
watcher);
+ folderKeys.put(k, d);
+ LOG.debug("Watching new directory: {}", d);
+ return FileVisitResult.CONTINUE;
+ }
+
+ @Override
+ public FileVisitResult visitFile(Path f, BasicFileAttributes
attrs) {
+ changed.add(f.toFile());
+ return FileVisitResult.CONTINUE;
+ }
+ });
+ } catch (IOException e) {
+ LOG.warn("Cannot watch new directory: {} due to: {}. This
exception is ignored.", dir, e.getMessage(), e);
+ }
+ }
+
+ /**
+ * Whether the directory is the compile work directory (or inside it): the
class files the runtime writes there when
+ * it compiles a Java source are not changes to watch, and would otherwise
trigger a reload of their own, which
+ * compiles again, which writes again (CAMEL-24862).
+ */
+ protected boolean isCompileWorkDir(Path dir) {
+ CompileStrategy cs = getCamelContext() != null
+ ?
getCamelContext().getCamelContextExtension().getContextPlugin(CompileStrategy.class)
: null;
+ String workDir = cs != null ? cs.getWorkDir() : null;
+ if (workDir == null) {
+ return false;
+ }
+ Path work = Paths.get(workDir).toAbsolutePath().normalize();
Review Comment:
⚠️ **Hot-path repeated work:** `isCompileWorkDir()` is called on every file
event (and on every directory `ENTRY_CREATE` event). Inside the method,
`getCamelContext().getCamelContextExtension().getContextPlugin(CompileStrategy.class)`
is resolved on each call, and
`Paths.get(workDir).toAbsolutePath().normalize()` constructs a new `Path`
object every time.
Since the compile work directory is immutable at runtime (set once before
the watcher starts), this should be resolved once in `doStart()` and stored as
a nullable field, e.g. `private Path compileWorkDirPath`. The check in
`isCompileWorkDir()` then becomes a single `startsWith` with no allocation per
event.
##########
core/camel-support/src/main/java/org/apache/camel/support/FileWatcherResourceReloadStrategy.java:
##########
@@ -225,13 +231,62 @@ private void registerRecursive(final WatchService
watcher, final Path root, fina
Files.walkFileTree(root, new SimpleFileVisitor<>() {
@Override
public FileVisitResult preVisitDirectory(Path dir,
BasicFileAttributes attrs) throws IOException {
+ if (isCompileWorkDir(dir)) {
+ return FileVisitResult.SKIP_SUBTREE;
+ }
WatchKey key = registerPathToWatcher(modifier, dir, watcher);
folderKeys.put(key, dir);
return FileVisitResult.CONTINUE;
}
});
}
+ /**
+ * Registers a directory created while watching recursively, and its
subdirectories, and collects the files already
+ * in them as changes: a tree such as src/main/java/com/acme is usually
created with its first file in it, before
+ * the watcher can see the directory.
+ */
+ private void registerNewDirectory(Path dir, List<File> changed) {
+ try {
+ Files.walkFileTree(dir, new SimpleFileVisitor<>() {
+ @Override
+ public FileVisitResult preVisitDirectory(Path d,
BasicFileAttributes attrs) throws IOException {
+ if (isCompileWorkDir(d) || folderKeys.containsValue(d)) {
+ return FileVisitResult.SKIP_SUBTREE;
+ }
+ WatchKey k = registerPathToWatcher(watchModifier, d,
watcher);
+ folderKeys.put(k, d);
+ LOG.debug("Watching new directory: {}", d);
+ return FileVisitResult.CONTINUE;
+ }
+
+ @Override
+ public FileVisitResult visitFile(Path f, BasicFileAttributes
attrs) {
+ changed.add(f.toFile());
+ return FileVisitResult.CONTINUE;
+ }
+ });
+ } catch (IOException e) {
+ LOG.warn("Cannot watch new directory: {} due to: {}. This
exception is ignored.", dir, e.getMessage(), e);
+ }
+ }
+
+ /**
+ * Whether the directory is the compile work directory (or inside it): the
class files the runtime writes there when
+ * it compiles a Java source are not changes to watch, and would otherwise
trigger a reload of their own, which
+ * compiles again, which writes again (CAMEL-24862).
+ */
+ protected boolean isCompileWorkDir(Path dir) {
+ CompileStrategy cs = getCamelContext() != null
+ ?
getCamelContext().getCamelContextExtension().getContextPlugin(CompileStrategy.class)
: null;
+ String workDir = cs != null ? cs.getWorkDir() : null;
+ if (workDir == null) {
+ return false;
+ }
+ Path work = Paths.get(workDir).toAbsolutePath().normalize();
Review Comment:
🔧 **Style:** `Paths.get()` is the legacy NIO API (deprecated in spirit since
Java 11). Use `Path.of()` instead — no extra import needed.
```suggestion
Path work = Path.of(workDir).toAbsolutePath().normalize();
```
##########
core/camel-support/src/main/java/org/apache/camel/support/FileWatcherResourceReloadStrategy.java:
##########
@@ -225,13 +231,62 @@ private void registerRecursive(final WatchService
watcher, final Path root, fina
Files.walkFileTree(root, new SimpleFileVisitor<>() {
@Override
public FileVisitResult preVisitDirectory(Path dir,
BasicFileAttributes attrs) throws IOException {
+ if (isCompileWorkDir(dir)) {
+ return FileVisitResult.SKIP_SUBTREE;
+ }
WatchKey key = registerPathToWatcher(modifier, dir, watcher);
folderKeys.put(key, dir);
return FileVisitResult.CONTINUE;
}
});
}
+ /**
+ * Registers a directory created while watching recursively, and its
subdirectories, and collects the files already
+ * in them as changes: a tree such as src/main/java/com/acme is usually
created with its first file in it, before
+ * the watcher can see the directory.
+ */
+ private void registerNewDirectory(Path dir, List<File> changed) {
+ try {
+ Files.walkFileTree(dir, new SimpleFileVisitor<>() {
+ @Override
+ public FileVisitResult preVisitDirectory(Path d,
BasicFileAttributes attrs) throws IOException {
+ if (isCompileWorkDir(d) || folderKeys.containsValue(d)) {
Review Comment:
🔍 **O(n) lookup:** `folderKeys.containsValue(d)` is a linear scan across all
registered directories. For deep watched trees this runs once per node during
`registerNewDirectory`'s recursive walk. An inverse `Set<Path>` (or a second
`Map<Path, WatchKey>`) would reduce this to O(1). Low severity since directory
creation events are rare, but worth addressing if the watcher is used on large
source trees.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
