This is an automated email from the ASF dual-hosted git repository.
ashishvijaywargiya pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-plugins.git
The following commit(s) were added to refs/heads/trunk by this push:
new a8f8539ae devreload dev notes, optmisation (#326)
a8f8539ae is described below
commit a8f8539ae40abf7708719516966931436603a181
Author: Ashish Vijaywargiya <[email protected]>
AuthorDate: Sat Jul 11 15:51:39 2026 +0530
devreload dev notes, optmisation (#326)
Adding dev notes in the bottom part of the README.md files.
Done code optimisation.
Split the code into multiple files.
---
devreload/README.md | 240 ++++++++++++
.../java/org/apache/ofbiz/devreload/Debouncer.java | 77 ++++
.../apache/ofbiz/devreload/DevReloadContainer.java | 409 ++++++++++++---------
.../ofbiz/devreload/RecordingFileManager.java | 54 +++
4 files changed, 604 insertions(+), 176 deletions(-)
diff --git a/devreload/README.md b/devreload/README.md
index 246d9495c..9df8d5243 100644
--- a/devreload/README.md
+++ b/devreload/README.md
@@ -54,3 +54,243 @@ Scope to specific components for a faster startup:
```
./gradlew ofbizDev --no-watch-fs -Photreload.components=devreload,party
```
+
+---
+# devreload — Dev Notes
+
+`devreload` is a development-only OFBiz plugin that removes the restart step
+from the edit → test loop. Save a `.java` file or a `services.xml` file and
+the change is live in well under a second, in the already-running OFBiz
+process.
+
+It lives at `plugins/devreload` and is completely self-contained: dropping the
+folder into a checkout adds the feature, deleting it removes the feature,
+with zero effect on the rest of OFBiz either way.
+
+---
+
+## 1. The problem it solves
+
+Without `devreload`, changing one line of Java meant:
+
+```
+edit .java → ./gradlew classes → kill OFBiz → wait 30-60s → restart → log in
again → test
+```
+
+With `devreload` running (`./gradlew ofbizDev --no-watch-fs`):
+
+```
+edit .java → save → compiled automatically → live in < 1s → test
+edit *services.xml → save → picked up automatically → live in < 1s → test
+```
+
+No restart, no re-login, no second terminal.
+
+---
+
+## 2. Who it's for / non-technical summary
+
+| Question | Answer |
+|---|---|
+| Who benefits? | Any developer actively writing/debugging OFBiz Java
services, event handlers, or `services.xml` files |
+| Does it affect production? | No. It is completely inert unless started with
a special flag (`-Dofbiz.hotreload=true`). Safe to have installed anywhere |
+| Does it change OFBiz core code? | No. The current design needs zero changes
to `ofbiz-framework` — it's a plugin only |
+| What do I run? | One command: `./gradlew ofbizDev --no-watch-fs` |
+| What do I need installed? | A DCEVM-patched JVM (e.g. JetBrains Runtime,
bundled with IntelliJ IDEA) |
+
+---
+
+## 3. How it works (technical overview)
+
+Four small classes do all the work:
+
+| Class | Role |
+|---|---|
+| `DevReloadContainer` | Watches source/config directories, compiles changed
Java in-process, and triggers reload |
+| `HotSwapAgent` | A tiny self-attaching Java agent that gives
`DevReloadContainer` access to `Instrumentation.redefineClasses` — the same
mechanism an IDE debugger uses for HotSwap |
+| `Debouncer<T>` | Coalesces rapid-fire change notifications (a burst of
file-system events, or one compile producing several `.class` files) into a
single batched action; shared by all three watch paths below |
+| `RecordingFileManager` | Wraps the in-process compiler's file manager to
record exactly which `.class` files it wrote, so the reload step hot-swaps
precisely what was compiled instead of guessing from the source file's name |
+
+At startup, `DevReloadContainer` self-attaches `HotSwapAgent` to the running
+JVM (via the Attach API — no `-javaagent` flag needed). This grants access to
+`Instrumentation`, which can replace the bytecode of an already-loaded class
+in place. Because the `Class` object's identity never changes, every existing
+reference to it (including OFBiz's own internal caches) automatically starts
+running the new code on the very next call — no framework code needs to know
+this plugin exists.
+
+### The three watch paths
+
+| Path | Watches | On change | Result |
+|---|---|---|---|
+| 1. Java source | Every component's `src/main/java` | Compiles the file
in-process, then hot-swaps the resulting class(es) | New method bodies live in
< 1s |
+| 2. `services.xml` | Every component's `servicedef/` directory | Clears the
`service.ModelServiceMapByModel` cache | OFBiz re-reads service definitions on
the next call |
+| 3. Build output (opt-in) | `build/classes/java/main` | Same hot-swap as Path
1 | Picks up `./gradlew -t classes` run in a second terminal |
+
+Changes are debounced for 300ms so a burst of related file writes (e.g. one
+compile producing several `.class` files) is handled as a single batch.
+
+### Method-body edits vs. structural changes
+
+| Change type | Stock JVM | DCEVM-patched JVM |
+|---|---|---|
+| Method body edit | ✅ Hot-swaps live | ✅ Hot-swaps live |
+| New/removed method or field, changed signature ("structural change") | ❌
Requires restart | ✅ Hot-swaps live |
+| Changed class hierarchy (superclass/interfaces) | ❌ Requires restart | ⚠️
Often hot-swaps live for classes with no existing instances (e.g. static-only
service/event classes) — support varies by DCEVM build and isn't guaranteed;
when the JVM can't apply it, that one class is rejected (see below), not
silently broken |
+
+Because structural changes are common during real development, `./gradlew
+ofbizDev` **requires** a DCEVM-patched JVM and refuses to start without one —
+rather than silently running in a degraded mode that "mostly" works.
+
+Whether a given hierarchy change hot-swaps depends on the specific DCEVM build
and
+on whether instances of the class already exist — OFBiz's service/event
classes are
+static-only with no live instances, which is the case DCEVM handles best. When
a
+class's redefinition genuinely can't be applied, only that class is rejected
(logged
+by name); every other valid change saved in the same batch still applies. A
rejected
+class stays "stuck" against that diff until OFBiz restarts.
+
+---
+
+## 4. What does and doesn't hot-reload
+
+| Works without restart | Needs a restart |
+|---|---|
+| Java method body changes | New OFBiz components (discovered at startup only)
|
+| New services / parameter changes in `services.xml` | `*UiLabels.xml` (loaded
at startup) |
+| New/removed methods, fields, changed signatures (DCEVM only) | `web.xml`,
`*.properties` files |
+| `controller.xml` (re-read every ~10s automatically — not tied to devreload
at all) | `entitymodel*.xml` changes |
+| | Changed class hierarchy (unreliable — often works for static-only classes
with no instances, but isn't guaranteed) |
+
+---
+
+## 5. Usage
+
+| Command | Effect |
+|---|---|
+| `./gradlew ofbizDev --no-watch-fs` | Start OFBiz with hot-reload enabled
(the only supported way to run it) |
+| `-Photreload.components=compA,compB` | Only watch these components — useful
on a large checkout to stay under the OS's directory-watch limit |
+| `-Photreload.watchBuildOutput=true` | Also watch Gradle's own build output
for externally-compiled classes |
+| `-PdcevmHome=/path/to/jvm` or `DCEVM_HOME` env var | Points at the
DCEVM-patched JVM (not auto-detected, by design) |
+
+`--no-watch-fs` disables Gradle's own file-watching, which otherwise competes
+with `devreload`'s own watcher for the same OS-level directory-watch budget
+(most noticeable on macOS on a full checkout).
+
+---
+
+## 6. Requirements
+
+| Requirement | Why |
+|---|---|
+| A JDK, not just a JRE | In-process compilation uses
`javax.tools.JavaCompiler` (`ToolProvider.getSystemJavaCompiler()`), which is
only present in a full JDK. Running on a JRE disables Java auto-compilation (a
warning is logged); `services.xml` reload still works |
+| A DCEVM-patched JVM | Required by the `ofbizDev` Gradle task specifically
(not by `DevReloadContainer` itself) so that structural changes (new/removed
methods/fields, changed signatures) hot-swap instead of silently requiring an
unannounced restart. Easiest source: JetBrains Runtime (JBR), bundled with
IntelliJ IDEA under `<IDE install>/jbr` (`.../Contents/jbr` on macOS) |
+| `-Djdk.attach.allowAttachSelf=true` | A JDK 9+ safeguard against a process
attaching to itself; required for `HotSwapAgent` to self-attach. Set
automatically by `./gradlew ofbizDev` |
+
+---
+
+## 7. System properties, Gradle properties, and paths
+
+Everything `devreload` reads or writes, in one place.
+
+| Property / path | Set by | Default | Purpose |
+|---|---|---|---|
+| `-Dofbiz.hotreload=true` | `ofbizDev` task (automatic) | unset (disabled) |
Master on/off switch. Absent or not `"true"` → `DevReloadContainer.init()`
returns immediately, fully inert |
+| `-Djdk.attach.allowAttachSelf=true` | `ofbizDev` task (automatic) | unset |
Lets `HotSwapAgent` self-attach via the Attach API |
+| `-Dofbiz.hotreload.components` | `-Photreload.components=compA,compB` |
unset (watch every component) | Comma-separated component names to scope
watching to, so the total watched-directory count fits under the OS's
per-process ceiling |
+| `-Dofbiz.hotreload.watchBuildOutput` | `-Photreload.watchBuildOutput=true` |
`false` | Whether `build/classes/java/main` is also watched, to pick up
externally-produced `.class` files (e.g. `./gradlew -t classes` in a second
terminal) |
+| `-Dofbiz.hotreload.outputDir` | `ofbizDev` task (automatic) |
`build/devreload/classes` | This plugin's own compiled-output directory.
Cleared and recreated on every start; placed ahead of Gradle's own output on
the runtime classpath |
+| `-XX:+AllowEnhancedClassRedefinition` | `ofbizDev` task, only once a DCEVM
JVM is resolved | not set on a stock JVM | Lifts the stock-JVM restriction so
`redefineClasses` also accepts structural changes |
+| `-PdcevmHome=/path/to/jvm` | manual, per invocation | unset | One-off
override pointing at a DCEVM-patched JVM's home directory |
+| `DCEVM_HOME` (env var) | manual, shell profile | unset | Same as
`-PdcevmHome`, but persisted so every future `./gradlew ofbizDev` picks it up
automatically |
+| `dcevmHome` (in `~/.gradle/gradle.properties`) | manual, one-time | unset |
Same idea as `DCEVM_HOME` but as a Gradle property instead of an env var — note
the different casing/name, the two are not interchangeable |
+| `build/classes/java/main` | Gradle (read-only from `devreload`'s
perspective) | — | Gradle's normal compiled-output directory; only ever
watched, never written to by this plugin |
+| `build/devreload/classes` | `devreload` itself | — | This plugin's private
compiled-output overlay; always wins over `build/classes/java/main` when a
class exists in both |
+
+---
+
+## 8. Repository layout
+
+| Repo | Contains | Why |
+|---|---|---|
+| `plugins/devreload` (a separate repo, dropped into a local checkout) |
`DevReloadContainer.java`, `HotSwapAgent.java`, `Debouncer.java`,
`RecordingFileManager.java`, `ofbiz-component.xml`, `build.gradle` (the
`ofbizDev` task), `README.md` — everything | OFBiz plugins are conventionally
distributed as separate repos; `ofbiz-framework`'s `.gitignore` excludes
`/plugins/` for exactly this reason |
+| `ofbiz-framework` | Nothing — no files, no diffs | The current design
redefines already-loaded `Class` objects in place, so no framework code ever
needs to ask "is a fresher class available." (An earlier prototype *did* need a
small reflective bridge into `framework/base` — see the historical entries in
the bug-fix table below for why that approach was replaced.) |
+
+To use it in a checkout: `git clone <devreload-repo-url> plugins/devreload`,
+then run `./gradlew ofbizDev --no-watch-fs`. Deleting `plugins/devreload/` at
+any time removes the feature completely, with zero effect on the rest of
+OFBiz.
+
+---
+
+## 9. Key design decisions
+
+| Decision | Why |
+|---|---|
+| `Instrumentation`-based class redefinition instead of a custom classloader |
Mutates the existing `Class` object in place — no second classloader to track,
no cache invalidation, no framework code changes needed |
+| Own output directory (`build/devreload/classes`), separate from Gradle's |
Writing into Gradle's managed output could confuse its incremental-build cache
and leave stale bytecode behind after a later `git checkout` |
+| Overlay directory always wins over Gradle's output when both exist | Simple,
predictable rule; matches its position on the runtime classpath |
+| One shared `Debouncer` class for all three watch paths | Avoids three
hand-rolled copies of the same concurrency logic that could drift out of sync |
+| Explicit `-PdcevmHome`/`DCEVM_HOME` only, no auto-detection | Guessing IDE
install paths breaks silently whenever a vendor changes packaging; one explicit
input is easier to keep working |
+| Failed directory watch = log a warning and skip it | No hidden fallback
(like polling); a clear, actionable warning instead of silent degraded behavior
|
+
+---
+
+## 10. Troubleshooting
+
+| Symptom | Likely cause | Fix |
+|---|---|---|
+| A directory logs a "could not watch" warning, unscoped on a full checkout |
OS directory-watch ceiling (e.g. macOS kqueue), possibly competing with
Gradle's own file watching | `./gradlew ofbizDev --no-watch-fs`, and/or
`-Photreload.components=compA,compB` to narrow the watched set |
+| Log: "DevReloadContainer is disabled" | Missing `-Dofbiz.hotreload=true` |
Use `./gradlew ofbizDev` (not `./gradlew ofbiz`) |
+| `ofbizDev` task doesn't exist | `plugins/devreload` not present, or missing
its `build.gradle` | `git clone` the plugin repo into `plugins/devreload` |
+| Log: "classes directory not found" | `./gradlew classes` not run yet |
`ofbizDev` runs `classes` automatically via `dependsOn` |
+| Log: "JavaCompiler not available" | Running on a JRE, not a JDK | Install a
JDK; use `./gradlew -t classes` in a second terminal as a fallback |
+| Log: "compilation failed — fix the error and save again" | Syntax/type error
in the saved `.java` file | The log line is followed by the real compiler
diagnostics (`file:line: message`); fix the reported error and save again — the
reload fires automatically |
+| Log: "could not self-attach HotSwapAgent" | Missing
`-Djdk.attach.allowAttachSelf=true` | Set automatically by `./gradlew
ofbizDev`; add it explicitly if starting OFBiz another way |
+| `services.xml` change not picked up | OFBiz restarted without going through
`ofbizDev` | Only works when `-Dofbiz.hotreload=true` is set |
+| A "just a method body" change still asks for a restart |
Structural-change-only edit (new/removed method or field, changed signature)
running on a stock JDK | Restart, or run on a DCEVM-patched JVM |
+| Could not find a DCEVM-patched JVM | Neither `-PdcevmHome` nor `DCEVM_HOME`
is set | Point at a JetBrains Runtime (`<IDE install>/jbr`) or standalone DCEVM
build explicitly — not auto-detected |
+| Same class keeps reporting a structural-change warning even after a pure
method-body edit | Once a stock JVM rejects one structural change on a class,
that class stays "stuck" until restart — every subsequent diff against its
still-loaded old bytecode still includes the pending change | Expected JVM
`redefineClasses` behavior, not a bug; restart to clear it |
+| Log: "batch redefinition rejected (...) -- retrying each class
individually", but only some classes in the save show "Hot-reload complete" |
One class in a debounced batch has a change the JVM can't apply; the others
were only rejected as part of the same all-or-nothing batch call | Expected
fallback behavior, not a bug — every class the JVM *can* apply still succeeds
individually; only the named, rejected class needs a restart |
+| `<attribute>`/`<override>` schema warning (`cvc-complex-type.2.4.a`) repeats
on every reload cycle | `<attribute>` elements must come before `<override>`
per `services.xsd` | Reorder the elements in the edited `services.xml` |
+
+---
+
+## 11. Top 25 bug fixes
+
+This component went through an earlier prototype design (a custom classloader
+approach, living directly in `framework/base`) before settling on the current
+`Instrumentation`-based design (a self-contained plugin). Fixes 1-14 are part
+of the current, shipped design. Fixes 15-20 were lessons learned in the
+earlier prototype; most of the mechanisms they touched no longer exist in the
+current design (noted per row), but the underlying lesson is worth keeping.
+Fixes 21-25 were found via later code-review passes on the current, shipped
+design, after real-world use surfaced edge cases the original test scenarios
+didn't happen to exercise.
+
+| # | Bug | Impact if unfixed | Fix |
+|---|---|---|---|
+| 1 | Directory-watch failures on a full checkout crashed the entire OFBiz
startup (`NoClassDefFoundError` from an unguarded fallback path) | OFBiz
wouldn't start at all on a large, unscoped checkout | Catch failures
per-directory and log a warning (`warnUnwatched`) instead of letting one bad
directory abort the whole watch setup |
+| 2 | Gradle's own file-system watcher competed with `devreload`'s watcher for
the same OS directory-watch ceiling (macOS kqueue limit) | Directories silently
went unwatched on a full checkout, even after fix #1 | Documented and defaulted
to `--no-watch-fs` for the `ofbizDev` command |
+| 3 | *(superseded by fix #23)* Inner and anonymous classes (e.g.
`Foo$1.class`) weren't included when hot-reloading their outer class |
Lambdas/anonymous classes kept running stale logic after a save | Originally
fixed by scanning the compiled output for every `Outer.class` and
`Outer$*.class` file, derived from the source file's own base name; superseded
by fix #23's more general approach |
+| 4 | Deleting a source file could trigger a reload attempt against a
now-missing class | Confusing errors / crash on file deletion | `ENTRY_DELETE`
events are explicitly ignored everywhere in the watch pipeline |
+| 5 | A cache-clear step after class redefinition could throw an `Error` (not
just an `Exception`), leaving the class updated but the service cache stale |
Inconsistent state: new bytecode active, but old service definitions still
cached | Widened the catch block to `catch (Throwable)` |
+| 6 | `IllegalArgumentException: 'other' is different type of Path` when
comparing an absolute path against a relative one | Every successful compile
failed to trigger a reload | Kept all paths consistently relative throughout
the compile/reload pipeline |
+| 7 | Watch keys became invalid after the in-process compiler wrote new class
files | Hot-reload would stop working after the very first compile |
Re-register the compiled-output directory at the end of every compile |
+| 8 | Shutting down while a compile finished could throw
`ClosedWatchServiceException`, logged as a scary "compilation error" |
Confusing noise in the logs on a completely normal shutdown | Catch `Exception`
broadly (this exception isn't an `IOException`) around that step |
+| 9 | Stopping the container while the watch thread was mid-event could throw
`RejectedExecutionException` when scheduling a reload | Confusing stack trace
during shutdown | Wrapped the scheduling call in a try/catch for this specific
case |
+| 10 | A single macOS `WatchService` event storm (one compiled file triggering
change events for the *entire* class tree) caused hundreds of unrelated classes
to be reloaded on every save | Noticeable slowdown on every save on macOS |
Unified all reload triggers through one shared debouncer, so a storm just
causes some harmless, bytecode-identical redundant reloads instead of a
performance hit |
+| 11 | A component with a `null` root location (no `rootLocation()`) threw a
`NullPointerException` while resolving its `src/main/java` path | Startup crash
— no source directories registered at all | Added a null guard before resolving
the source path in `registerSourceDirs()` |
+| 12 | An invalid/deleted watch key (`key.reset()` returning `false`) was
silently ignored | WatchService could quietly stop detecting changes in a
directory with no indication why | Check the return value and log a warning
naming the directory |
+| 13 | A `NoClassDefFoundError` (or similar) while registering one component's
`servicedef` directory could abort the loop, leaving every later component's
directory unwatched | Only the first few components' `services.xml` changes
would ever be detected | Wrapped each component's registration in its own
`catch (Throwable)` so one failure never blocks the rest |
+| 14 | Comparing modification times across two output directories (overlay vs.
Gradle's) to decide which compiled class "wins," including deleting the stale
copy when Gradle's was newer | Subtle correctness risk: a timestamp comparison
plus a reconciling side-effect (file deletion) that could easily fall out of
sync with the classpath's actual resolution order | Simplified to one rule: the
overlay directory always wins when a copy exists there — consistent with its
fixed position on the [...]
+| 15 | *(historical, superseded)* Multi-cycle reload loss — editing file B
produced a classloader that only knew about B; class A (changed in an earlier
reload) fell back to its stale, startup-time version | Any class changed more
than one reload ago silently regressed to old behavior | Prototype-only fix (an
`allChangedClasses` accumulator); not applicable to the current design, since
`redefineClasses` always applies the latest compiled bytecode directly with no
classloader to lose trac [...]
+| 16 | *(historical, superseded)* `ClassFormatError` (a `LinkageError`)
escaped as an unhandled `Error` when a class was read mid-write by a racing
compiler | Unhandled `Error` could crash a request in the servlet container |
Prototype-only fix (catch `LinkageError` during classload); not applicable now
— `applyCompile()` reads its own compiler output synchronously, so there's no
separate framework-side read racing the write |
+| 17 | *(historical, superseded)* Used
`Thread.currentThread().getContextClassLoader()` as a parent reference from a
background thread | Silent class-resolution failures if the servlet container
mutated the thread's context classloader | Prototype-only fix (switched to a
deterministic classloader reference); not applicable now — there's no custom
classloader in the current design at all |
+| 18 | *(historical)* A code comment claimed
`synchronized(DevReloadContainer.class)` while the actual code used
`synchronized(this)` | A future developer trusting the comment could introduce
a real data race | Comment/code mismatch fixed by reverting to a `private
final` instance field with unambiguous `synchronized(this)` |
+| 19 | *(historical)* A single save produced two reload attempts for the same
class — one direct call plus one from the WatchService event for the same
`.class` file | Harmless but wasteful; duplicate "Hot-reload complete" log
lines on every save | Originally patched with a timing-based suppression
window; the current design's real fix is architectural (fix #10) — one shared
debouncer per change type, so a duplicate at most causes one redundant,
harmless redefinition |
+| 20 | *(historical)* Hardcoded, per-OS guesses at common IDE install paths to
auto-detect a DCEVM-patched JVM (IntelliJ on macOS/Linux/Windows, JetBrains
Toolbox, etc.) | Silently broke for any install layout not on the guessed list
— a "works for some users, not others" pattern that grew another special case
with every new packaging variant | Removed all guessing in favor of one
explicit, always-honored input: `-PdcevmHome` or `DCEVM_HOME` |
+| 21 | A single unsupported structural change in a debounced batch caused
`Instrumentation.redefineClasses` to reject the *entire* batch in one call,
since every changed class's `ClassDefinition` was passed together | An
unrelated, valid method-body edit that happened to land in the same ~300ms
debounce window as a rejected class silently failed to apply too, with nothing
in the log distinguishing it from the actual offender |
`redefineWithPerClassFallback` tries the batch call first (th [...]
+| 22 | `applyCompile()` passed a `null` `DiagnosticListener` to
`javax.tools.JavaCompiler`, so a failed in-process compile produced no detail
about what actually failed | The log's only output on a broken save was the
generic "compilation failed — fix the error and save again", with no file, line
number, or compiler message to act on | Pass a
`DiagnosticCollector<JavaFileObject>` to the compiler task and log each
`ERROR`-level diagnostic (`file:line: message`) alongside the existing warning |
+| 23 | The post-compile reload step guessed which `.class` files to hot-swap
from the *source file's own base name* (`Outer.class`/`Outer$*.class`) | A
secondary top-level class in the same source file, or a lone non-public
top-level class named differently from its file (both legal Java), compiled to
disk correctly but was never queued for redefinition — it silently kept running
stale bytecode while the log claimed "Hot-reload complete" | Wrap the
compiler's file manager in `RecordingFi [...]
+| 24 | In-process compilation read source files using
`Charset.defaultCharset()` (passed `null` to `getStandardFileManager`) | The
project's real `compileJava` forces `options.encoding = 'UTF-8'` (root
`build.gradle`); on any JVM/OS whose platform default charset isn't UTF-8
(pre-JDK18, or an overridden `-Dfile.encoding` — plausible on Windows), a
hot-reload compile of a file with non-ASCII characters (i18n literals,
non-English comments) decoded differently than Gradle's own compile, pr [...]
+| 25 | A brand-new subdirectory created with files already inside it in one
filesystem operation (e.g. a `git checkout` that adds a whole package, an IDE
"extract to new package" refactor, or unzipping a folder into a watched source
tree) could have those pre-existing files never picked up | The OS/JDK
`WatchService` doesn't retroactively report `ENTRY_CREATE` for files that
existed before their directory was registered, so such files silently never
compiled until each was individually r [...]
diff --git a/devreload/src/main/java/org/apache/ofbiz/devreload/Debouncer.java
b/devreload/src/main/java/org/apache/ofbiz/devreload/Debouncer.java
new file mode 100644
index 000000000..1d10a87cc
--- /dev/null
+++ b/devreload/src/main/java/org/apache/ofbiz/devreload/Debouncer.java
@@ -0,0 +1,77 @@
+/*******************************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+
*******************************************************************************/
+package org.apache.ofbiz.devreload;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.RejectedExecutionException;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+
+/**
+ * Coalesces rapid-fire change notifications into one action, so a single
compile run
+ * that touches many files (e.g. one with inner/anonymous classes, or a Gradle
build
+ * writing several {@code .class} files at once) is handled as a single batch
instead
+ * of one action per file. Shared by all three of {@link DevReloadContainer}'s
change
+ * pipelines (class reload, {@code services.xml} reload, Java compile) instead
of each
+ * hand-rolling its own pending-set/cancel/reschedule bookkeeping.
+ *
+ * <p>The backing executor is supplied lazily via {@code executorSupplier}
rather than
+ * captured directly: {@link DevReloadContainer} builds its three {@code
Debouncer}
+ * instances as field initializers, which run before its own executor is
created in
+ * {@code init()}.
+ */
+final class Debouncer<T> {
+ private final Supplier<ScheduledExecutorService> executorSupplier;
+ private final Set<T> pending = new HashSet<>();
+ private final Consumer<Set<T>> action;
+ private ScheduledFuture<?> scheduled;
+
+ Debouncer(Supplier<ScheduledExecutorService> executorSupplier,
Consumer<Set<T>> action) {
+ this.executorSupplier = executorSupplier;
+ this.action = action;
+ }
+
+ synchronized void add(T item) {
+ pending.add(item);
+ if (scheduled != null) {
+ scheduled.cancel(false);
+ }
+ try {
+ // Wait 300 ms after the last change so a burst of related changes
(e.g. a
+ // single Gradle compile run writing multiple .class files) is
handled as
+ // one batch instead of one action per file.
+ scheduled = executorSupplier.get().schedule(this::fire, 300,
TimeUnit.MILLISECONDS);
+ } catch (RejectedExecutionException e) {
+ // Container is shutting down; pending changes will not be applied.
+ }
+ }
+
+ private synchronized void fire() {
+ if (pending.isEmpty()) {
+ return;
+ }
+ Set<T> batch = new HashSet<>(pending);
+ pending.clear();
+ action.accept(batch);
+ }
+}
diff --git
a/devreload/src/main/java/org/apache/ofbiz/devreload/DevReloadContainer.java
b/devreload/src/main/java/org/apache/ofbiz/devreload/DevReloadContainer.java
index 1f9539cb2..5c82638f1 100644
--- a/devreload/src/main/java/org/apache/ofbiz/devreload/DevReloadContainer.java
+++ b/devreload/src/main/java/org/apache/ofbiz/devreload/DevReloadContainer.java
@@ -25,6 +25,7 @@ import java.lang.management.ManagementFactory;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
+import java.nio.charset.StandardCharsets;
import java.nio.file.ClosedWatchServiceException;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
@@ -38,17 +39,18 @@ import java.nio.file.WatchService;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
+import java.util.Map;
import java.util.Set;
import java.util.concurrent.Executors;
-import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ScheduledFuture;
-import java.util.concurrent.TimeUnit;
-import java.util.function.Consumer;
import java.util.stream.Collectors;
+import javax.tools.Diagnostic;
+import javax.tools.DiagnosticCollector;
import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
import javax.tools.StandardJavaFileManager;
import javax.tools.StandardLocation;
import javax.tools.ToolProvider;
@@ -146,15 +148,24 @@ public class DevReloadContainer implements Container {
private static final String MODULE = DevReloadContainer.class.getName();
private static final String SERVICE_MODEL_CACHE_NAME =
"service.ModelServiceMapByModel";
+ // Shared by start()'s aggregated warning and warnUnwatched()'s
per-directory warning,
+ // so the two can't silently drift apart if the property/flag names ever
change.
+ private static final String SCOPE_HINT =
"-Dofbiz.hotreload.components=compA,compB (or "
+ + "-Photreload.components=compA,compB with the ofbizDev Gradle
task) to fit under "
+ + "the OS watch limit.";
+
private String name;
private WatchService watchService;
private Thread watchThread;
private ScheduledExecutorService debounceExecutor;
private Instrumentation instrumentation;
- private final Debouncer<String> classReloadDebouncer = new
Debouncer<>(this::applyReload);
- private final Debouncer<Path> xmlReloadDebouncer = new
Debouncer<>(this::applyServiceXmlReload);
- private final Debouncer<Path> compileDebouncer = new
Debouncer<>(this::applyCompile);
+ // The executor is supplied lazily (a Supplier, not a direct reference)
because these
+ // fields are initialized before startDebounceExecutor() creates
debounceExecutor --
+ // see Debouncer's own javadoc.
+ private final Debouncer<String> classReloadDebouncer = new Debouncer<>(()
-> debounceExecutor, this::applyReload);
+ private final Debouncer<Path> xmlReloadDebouncer = new Debouncer<>(() ->
debounceExecutor, this::applyServiceXmlReload);
+ private final Debouncer<Path> compileDebouncer = new Debouncer<>(() ->
debounceExecutor, this::applyCompile);
// Counts across
registerServicedefDirs()/registerSourceDirs()/registerAll(), so
// start() can emit one aggregated warning instead of leaving individual
failures
@@ -191,6 +202,11 @@ public class DevReloadContainer implements Container {
*/
private Set<String> allowedComponents;
+ /** {@code true} when {@code componentName} should be watched: no scoping
set, or it's in the scoped set. */
+ private boolean isAllowedComponent(String componentName) {
+ return allowedComponents == null ||
allowedComponents.contains(componentName);
+ }
+
/**
* Whether {@link #classesDir} ({@code build/classes/java/main}) itself is
watched, from
* {@code -Dofbiz.hotreload.watchBuildOutput}; defaults to {@code false}.
This tree mirrors
@@ -327,10 +343,7 @@ public class DevReloadContainer implements Container {
try {
registerAll(classesDir);
} catch (IOException e) {
- // registerAll() already logs a warning and skips individual
directories
- // that fail to register; reaching here means something more
fundamental
- // broke walking the tree at all (e.g. can't even list
classesDir).
- Debug.logWarning("Hot-reload: could not fully walk " +
classesDir + ": " + e.getMessage(), MODULE);
+ logWalkFailure(classesDir, e);
}
Debug.logInfo("Hot-reload: watching compiled-output directory " +
classesDir.toAbsolutePath()
+ " for externally-produced .class files (set via
-Dofbiz.hotreload.watchBuildOutput=true).",
@@ -383,9 +396,7 @@ public class DevReloadContainer implements Container {
Debug.logWarning("Hot-reload: " + watchDirsFailed + " of " +
watchDirsAttempted + " directory watch "
+ "registrations hit file-descriptor/watch exhaustion (see
warnings above for which ones) "
+ "and are NOT being watched — changes there will not
hot-reload until you restart. Scope "
- + "hot-reload to just the components you're working on
with "
- + "-Dofbiz.hotreload.components=compA,compB (or
-Photreload.components=compA,compB with "
- + "the ofbizDev Gradle task) to fit under the OS watch
limit.", MODULE);
+ + "hot-reload to just the components you're working on
with " + SCOPE_HINT, MODULE);
}
watchThread = new Thread(this::watchLoop, "ofbiz-hot-reload-watcher");
watchThread.setDaemon(true);
@@ -401,7 +412,7 @@ public class DevReloadContainer implements Container {
*/
private void registerServicedefDirs() {
for (ComponentConfig.ServiceResourceInfo sri :
ComponentConfig.getAllServiceResourceInfos("model")) {
- if (allowedComponents != null &&
!allowedComponents.contains(sri.getComponentConfig().getComponentName())) {
+ if
(!isAllowedComponent(sri.getComponentConfig().getComponentName())) {
continue;
}
try {
@@ -410,16 +421,8 @@ public class DevReloadContainer implements Container {
continue; // skip non-filesystem resources (classpath
jars, etc.)
}
Path dir = Paths.get(new URI(url.toString())).getParent();
- if (dir != null && Files.isDirectory(dir) &&
servicedefDirs.add(dir)) {
- watchDirsAttempted++;
- try {
- dir.register(watchService,
- StandardWatchEventKinds.ENTRY_CREATE,
- StandardWatchEventKinds.ENTRY_MODIFY);
- Debug.logInfo("Hot-reload: watching servicedef
directory " + dir, MODULE);
- } catch (IOException e) {
- warnUnwatched(dir, e);
- }
+ if (dir != null && Files.isDirectory(dir) &&
servicedefDirs.add(dir) && registerWatch(dir)) {
+ Debug.logInfo("Hot-reload: watching servicedef directory "
+ dir, MODULE);
}
} catch (GenericConfigException | URISyntaxException e) {
Debug.logWarning("Hot-reload: could not register servicedef
dir for "
@@ -450,7 +453,7 @@ public class DevReloadContainer implements Container {
if (cc.rootLocation() == null) {
continue;
}
- if (allowedComponents != null &&
!allowedComponents.contains(cc.getComponentName())) {
+ if (!isAllowedComponent(cc.getComponentName())) {
continue;
}
Path srcDir = cc.rootLocation().resolve("src/main/java");
@@ -459,10 +462,7 @@ public class DevReloadContainer implements Container {
registerAll(srcDir);
Debug.logInfo("Hot-reload: watching source directory " +
srcDir, MODULE);
} catch (IOException e) {
- // registerAll() already logs a warning and skips
individual directories
- // that fail to register; reaching here means something
more fundamental
- // broke walking the tree at all (e.g. can't list srcDir).
- Debug.logWarning("Hot-reload: could not walk source dir "
+ srcDir + ": " + e.getMessage(), MODULE);
+ logWalkFailure(srcDir, e);
}
}
}
@@ -520,26 +520,23 @@ public class DevReloadContainer implements Container {
Path changed = dir.resolve(((WatchEvent<Path>)
event).context());
if (kind == StandardWatchEventKinds.ENTRY_CREATE &&
Files.isDirectory(changed)) {
- // New package directory created during compilation —
register it.
+ // New package directory (or a whole new subtree -- e.g. a
`git checkout`
+ // that adds a package, an IDE "extract to new package"
refactor, or
+ // unzipping a folder) -- register it AND dispatch any
files already inside
+ // it. The OS/JDK WatchService does not retroactively
report CREATE events
+ // for files that already existed before a directory was
registered, so
+ // without this seeding step, files that land here in the
same operation
+ // that created the directory would silently never compile
until each one
+ // is individually re-saved.
try {
- registerAll(changed);
+ registerAllAndSeed(changed);
} catch (IOException e) {
Debug.logError(e, "DevReloadContainer: failed to
register new directory: " + changed, MODULE);
}
- } else if ((kind == StandardWatchEventKinds.ENTRY_CREATE ||
kind == StandardWatchEventKinds.ENTRY_MODIFY)
- && changed.toString().endsWith(".class")) {
- // Only react to written/updated class files. Ignore
ENTRY_DELETE so
- // that removing a source file (and its .class output)
does not cause
- // a redefinition attempt against a now-missing file.
- reloadClassFile(classesDir, changed);
- } else if ((kind == StandardWatchEventKinds.ENTRY_CREATE ||
kind == StandardWatchEventKinds.ENTRY_MODIFY)
- && changed.toString().endsWith(".xml")
- && servicedefDirs.stream().anyMatch(dir::startsWith)) {
- xmlReloadDebouncer.add(changed);
- } else if ((kind == StandardWatchEventKinds.ENTRY_CREATE ||
kind == StandardWatchEventKinds.ENTRY_MODIFY)
- && changed.toString().endsWith(".java")
- && sourceRootDirs.stream().anyMatch(dir::startsWith)) {
- compileDebouncer.add(changed);
+ } else if (kind == StandardWatchEventKinds.ENTRY_CREATE ||
kind == StandardWatchEventKinds.ENTRY_MODIFY) {
+ // Ignore ENTRY_DELETE so that removing a source file (and
its .class
+ // output) does not cause a redefinition attempt against a
now-missing file.
+ dispatchChangedFile(dir, changed);
}
}
if (!key.reset()) {
@@ -549,52 +546,29 @@ public class DevReloadContainer implements Container {
}
}
- //
-------------------------------------------------------------------------
- // Debounced batching
- //
-------------------------------------------------------------------------
-
/**
- * Coalesces rapid-fire change notifications into one action, so a single
compile run
- * that touches many files (e.g. one with inner/anonymous classes, or a
Gradle build
- * writing several {@code .class} files at once) is handled as a single
batch instead
- * of one action per file. Shared by all three change pipelines (class
reload,
- * {@code services.xml} reload, Java compile) instead of each hand-rolling
its own
- * pending-set/cancel/reschedule bookkeeping.
+ * Routes a single created/modified regular file through the correct
reload pipeline --
+ * a {@code .class} file under {@link #classesDir}, a {@code .xml} file
under a
+ * registered servicedef directory, or a {@code .java} file under a
registered source
+ * root. Shared between {@link #watchLoop}'s live WatchService events and
+ * {@link #registerAllAndSeed}'s seeding of files that already existed
when a new
+ * directory was first discovered.
*/
- private final class Debouncer<T> {
- private final Set<T> pending = new HashSet<>();
- private final Consumer<Set<T>> action;
- private ScheduledFuture<?> scheduled;
-
- Debouncer(Consumer<Set<T>> action) {
- this.action = action;
- }
-
- synchronized void add(T item) {
- pending.add(item);
- if (scheduled != null) {
- scheduled.cancel(false);
- }
- try {
- // Wait 300 ms after the last change so a burst of related
changes (e.g. a
- // single Gradle compile run writing multiple .class files) is
handled as
- // one batch instead of one action per file.
- scheduled = debounceExecutor.schedule(this::fire, 300,
TimeUnit.MILLISECONDS);
- } catch (RejectedExecutionException e) {
- // Container is shutting down; pending changes will not be
applied.
- }
- }
-
- private synchronized void fire() {
- if (pending.isEmpty()) {
- return;
- }
- Set<T> batch = new HashSet<>(pending);
- pending.clear();
- action.accept(batch);
+ private void dispatchChangedFile(Path dir, Path changed) {
+ String name = changed.toString();
+ if (name.endsWith(".class")) {
+ reloadClassFile(classesDir, changed);
+ } else if (name.endsWith(".xml") &&
servicedefDirs.stream().anyMatch(dir::startsWith)) {
+ xmlReloadDebouncer.add(changed);
+ } else if (name.endsWith(".java") &&
sourceRootDirs.stream().anyMatch(dir::startsWith)) {
+ compileDebouncer.add(changed);
}
}
+ //
-------------------------------------------------------------------------
+ // Debounced batching
+ //
-------------------------------------------------------------------------
+
/** Resolves {@code changed} to a class name relative to {@code baseDir}
and, if valid, queues it for reload. */
private void reloadClassFile(Path baseDir, Path changed) {
String className = toClassName(baseDir, changed);
@@ -612,6 +586,8 @@ public class DevReloadContainer implements Container {
return;
}
+ Map<String, Class<?>> loadedByName = indexLoadedClasses(batch);
+
List<ClassDefinition> defs = new ArrayList<>();
for (String className : batch) {
Path relative = Paths.get(className.replace('.', '/') + ".class");
@@ -624,15 +600,15 @@ public class DevReloadContainer implements Container {
+ hotReloadOutputDir.toAbsolutePath() + " or " +
classesDir.toAbsolutePath(), MODULE);
continue;
}
+ Class<?> loaded = loadedByName.get(className);
+ if (loaded == null) {
+ // Never loaded yet in this JVM — nothing to redefine. It will
simply
+ // load fresh, with the new bytecode, the first time something
+ // references it, from whichever directory resolveClassFile()
would
+ // pick (overlay first on the classpath too, see build.gradle).
+ continue;
+ }
try {
- Class<?> loaded = findLoadedClass(className);
- if (loaded == null) {
- // Never loaded yet in this JVM — nothing to redefine. It
will simply
- // load fresh, with the new bytecode, the first time
something
- // references it, from whichever directory
resolveClassFile() would
- // pick (overlay first on the classpath too, see
build.gradle).
- continue;
- }
defs.add(new ClassDefinition(loaded,
Files.readAllBytes(classFile)));
} catch (IOException e) {
Debug.logError(e, "Hot-reload: failed to read class file for "
+ className, MODULE);
@@ -644,6 +620,36 @@ public class DevReloadContainer implements Container {
return;
}
+ redefineWithPerClassFallback(defs);
+ }
+
+ /**
+ * Builds a {@code className -> Class} map covering just the classes in
{@code batch},
+ * scanning {@link Instrumentation#getAllLoadedClasses} once instead of
once per class
+ * name — that array holds every class loaded in the JVM (routinely tens
of thousands
+ * in a running OFBiz instance), so re-scanning it per class name in a
batch made every
+ * save pay for an O(batchSize * totalLoadedClasses) walk.
+ */
+ private Map<String, Class<?>> indexLoadedClasses(Set<String> batch) {
+ Map<String, Class<?>> byName = new HashMap<>();
+ for (Class<?> c : instrumentation.getAllLoadedClasses()) {
+ if (batch.contains(c.getName())) {
+ byName.put(c.getName(), c);
+ }
+ }
+ return byName;
+ }
+
+ /**
+ * Redefines every class in {@code defs} in one {@link
Instrumentation#redefineClasses}
+ * call when possible — the common, fast path. If the JVM rejects the
whole batch
+ * because one class contains a structural change it can't apply
+ * ({@link UnsupportedOperationException}), falls back to redefining each
class
+ * individually instead: otherwise, an unrelated valid method-body edit
that happened
+ * to land in the same debounced batch as an unsupported structural change
would
+ * silently fail to apply right along with it.
+ */
+ private void redefineWithPerClassFallback(List<ClassDefinition> defs) {
try {
instrumentation.redefineClasses(defs.toArray(new
ClassDefinition[0]));
// Clear service definition cache so newly added service methods
are discovered.
@@ -651,22 +657,52 @@ public class DevReloadContainer implements Container {
// has not changed, only .class files have, and clearing those
caches triggers
// Groovy re-compilation of screen expressions which can fail
unexpectedly.
UtilCache.clearCache(SERVICE_MODEL_CACHE_NAME);
- Debug.logInfo("Hot-reload complete for: " + batch, MODULE);
+ Debug.logInfo("Hot-reload complete for: " + classNames(defs),
MODULE);
+ return;
} catch (UnsupportedOperationException e) {
+ Debug.logWarning("Hot-reload: batch redefinition rejected (" +
e.getMessage()
+ + ") -- retrying each class individually so other, valid
changes in the "
+ + "same save still apply.", MODULE);
+ } catch (Throwable e) {
+ Debug.logError(e, "Hot-reload failed for " + classNames(defs),
MODULE);
+ return;
+ }
+
+ List<String> applied = new ArrayList<>();
+ List<String> rejected = new ArrayList<>();
+ for (ClassDefinition def : defs) {
+ String className = def.getDefinitionClass().getName();
+ try {
+ instrumentation.redefineClasses(def);
+ applied.add(className);
+ } catch (UnsupportedOperationException e) {
+ rejected.add(className);
+ } catch (Throwable e) {
+ Debug.logError(e, "Hot-reload failed for " + className,
MODULE);
+ rejected.add(className);
+ }
+ }
+ if (!applied.isEmpty()) {
+ UtilCache.clearCache(SERVICE_MODEL_CACHE_NAME);
+ Debug.logInfo("Hot-reload complete for: " + applied, MODULE);
+ }
+ if (!rejected.isEmpty()) {
// ./gradlew ofbizDev only ever runs on a DCEVM-patched JVM (see
build.gradle),
// which already lifts the plain-JVM restriction to method bodies
only, so
// add/remove method-or-field and signature changes normally
succeed here. This
// still fires for the narrower set of changes DCEVM itself can't
apply either
// (e.g. a changed class hierarchy) -- the same remaining limit an
IDE debugger's
// HotSwap has even on a capable JVM.
- Debug.logWarning("Hot-reload: " + batch + " contains a structural
change (added/removed "
+ Debug.logWarning("Hot-reload: " + rejected + " contain a
structural change (added/removed "
+ "method or field, changed signature, changed hierarchy)
that the JVM cannot "
- + "hot-swap. Restart OFBiz to pick it up. (" +
e.getMessage() + ")", MODULE);
- } catch (Throwable e) {
- Debug.logError(e, "Hot-reload failed for " + batch, MODULE);
+ + "hot-swap. Restart OFBiz to pick it up.", MODULE);
}
}
+ private static List<String> classNames(List<ClassDefinition> defs) {
+ return defs.stream().map(def ->
def.getDefinitionClass().getName()).collect(Collectors.toList());
+ }
+
/**
* Best-effort detection of whether this JVM was launched with
* {@code -XX:+AllowEnhancedClassRedefinition} (e.g. a JetBrains Runtime),
which is
@@ -680,16 +716,6 @@ public class DevReloadContainer implements Container {
.anyMatch(arg ->
arg.contains("AllowEnhancedClassRedefinition"));
}
- /** Searches classes already loaded in the JVM for one matching {@code
className}. */
- private Class<?> findLoadedClass(String className) {
- for (Class<?> c : instrumentation.getAllLoadedClasses()) {
- if (c.getName().equals(className)) {
- return c;
- }
- }
- return null;
- }
-
private void applyServiceXmlReload(Set<Path> batch) {
Debug.logInfo("Hot-reload: service XML changed " + batch + " —
clearing service model cache", MODULE);
try {
@@ -707,48 +733,36 @@ public class DevReloadContainer implements Container {
if (compiler == null) {
return;
}
- try (StandardJavaFileManager fm =
compiler.getStandardFileManager(null, null, null)) {
- fm.setLocation(StandardLocation.CLASS_OUTPUT,
+ DiagnosticCollector<JavaFileObject> diagnostics = new
DiagnosticCollector<>();
+ // Explicit UTF-8: the project's real compileJava forces
options.encoding = 'UTF-8'
+ // (root build.gradle). Passing null here would fall back to
Charset.defaultCharset(),
+ // which isn't guaranteed to be UTF-8 on every JVM/OS (notably
pre-JDK18, or any JVM
+ // launched with an overridden -Dfile.encoding) and would silently
decode source files
+ // containing non-ASCII characters differently than Gradle's own
compile does.
+ try (StandardJavaFileManager standardFm =
+ compiler.getStandardFileManager(diagnostics, null,
StandardCharsets.UTF_8)) {
+ standardFm.setLocation(StandardLocation.CLASS_OUTPUT,
List.of(hotReloadOutputDir.toAbsolutePath().toFile()));
+ RecordingFileManager fm = new RecordingFileManager(standardFm);
// Reuse the running JVM's classpath — it already contains all
OFBiz jars.
List<String> options = Arrays.asList("-cp",
System.getProperty("java.class.path"), "-proc:none");
- var units = fm.getJavaFileObjectsFromPaths(batch);
- boolean ok = compiler.getTask(null, fm, null, options, null,
units).call();
+ var units = standardFm.getJavaFileObjectsFromPaths(batch);
+ boolean ok = compiler.getTask(null, fm, diagnostics, options,
null, units).call();
if (ok) {
Debug.logInfo("Hot-reload: compilation successful",
MODULE);
- // Collect all .class files produced by this compilation
round.
- // Each source file can produce multiple .class files when
it contains
- // inner or anonymous classes (e.g. Foo$Bar.class,
Foo$1.class).
- // All of them must be redefined too — otherwise the inner
class still
- // resolves through its stale, previously-loaded bytecode.
- for (Path src : batch) {
- Path cf = sourceToClassFile(src); // relative path for
the outer class
- if (cf == null) {
- continue;
- }
- String outerName =
cf.getFileName().toString().replace(".class", "");
- Path absOutputDir = cf.toAbsolutePath().getParent();
- try (var dirStream = Files.list(absOutputDir)) {
- dirStream.filter(absFile -> {
- String fn = absFile.getFileName().toString();
- // Match Foo.class and Foo$Inner.class /
Foo$1.class
- return fn.endsWith(".class")
- && (fn.equals(outerName + ".class")
- || fn.startsWith(outerName +
"$"));
- }).forEach(absFile -> {
- // Convert absolute output path back to a
relative path that
- // is rooted at CWD (same type as
hotReloadOutputDir) so that
- // toClassName(hotReloadOutputDir, relPath) —
which calls
- // relativize — does not throw
IllegalArgumentException.
- Path rel = hotReloadOutputDir.resolve(
-
hotReloadOutputDir.toAbsolutePath().relativize(absFile));
- reloadClassFile(hotReloadOutputDir, rel);
- });
- } catch (IOException e) {
- // Output dir unreadable; fall back to the outer
class only.
- reloadClassFile(hotReloadOutputDir, cf);
- }
+ // Reload every .class file the compiler actually wrote
for this batch, per
+ // RecordingFileManager -- covers inner/anonymous classes
(Foo$1.class) and,
+ // unlike a name-based guess derived from the source
file's own base name,
+ // also a secondary top-level class in the same source
file, or a lone
+ // non-public top-level class named differently from its
file (both legal
+ // Java). A name-based guess would compile such a class to
disk correctly
+ // but never queue it for redefinition, leaving it
silently running stale
+ // bytecode.
+ for (Path absFile : fm.outputFiles) {
+ Path rel = hotReloadOutputDir.resolve(
+
hotReloadOutputDir.toAbsolutePath().relativize(absFile));
+ reloadClassFile(hotReloadOutputDir, rel);
}
// Re-register class directories so external compilations
(./gradlew classes
@@ -766,7 +780,8 @@ public class DevReloadContainer implements Container {
}
}
} else {
- Debug.logWarning("Hot-reload: compilation failed — fix the
error and save again", MODULE);
+ Debug.logWarning("Hot-reload: compilation failed — fix the
error and save again\n"
+ + formatErrors(diagnostics), MODULE);
}
}
} catch (Throwable e) {
@@ -774,23 +789,18 @@ public class DevReloadContainer implements Container {
}
}
- /**
- * Maps a {@code .java} source file to the corresponding {@code .class}
output file
- * under {@link #hotReloadOutputDir}. Returns {@code null} if the source
file is not
- * under any registered source root.
- */
- private Path sourceToClassFile(Path sourceFile) {
- for (Path srcRoot : sourceRootDirs) {
- if (sourceFile.startsWith(srcRoot)) {
- Path relative = srcRoot.relativize(sourceFile);
- String name = relative.toString();
- if (name.endsWith(".java")) {
- String classRelative = name.substring(0, name.length() -
".java".length()) + ".class";
- return hotReloadOutputDir.resolve(classRelative);
- }
+ /** Formats the error-level diagnostics from a failed compile, one per
line, as {@code file:line: message}. */
+ private static String formatErrors(DiagnosticCollector<JavaFileObject>
diagnostics) {
+ StringBuilder sb = new StringBuilder();
+ for (Diagnostic<? extends JavaFileObject> d :
diagnostics.getDiagnostics()) {
+ if (d.getKind() != Diagnostic.Kind.ERROR) {
+ continue;
}
+ String source = d.getSource() != null ? d.getSource().getName() :
"?";
+ sb.append("
").append(source).append(':').append(d.getLineNumber())
+ .append(": ").append(d.getMessage(null)).append('\n');
}
- return null;
+ return sb.toString();
}
//
-------------------------------------------------------------------------
@@ -804,27 +814,75 @@ public class DevReloadContainer implements Container {
* so one overloaded directory never leaves the rest of the tree unwatched.
*/
private void registerAll(Path start) throws IOException {
+ walkAndRegister(start, false);
+ }
+
+ /**
+ * Logs that {@link #registerAll} itself failed for {@code dir} --
something more
+ * fundamental broke walking the tree at all (e.g. can't even list {@code
dir}), as
+ * opposed to an individual directory failing to register, which {@code
registerAll()}
+ * already logs and skips on its own via {@link #warnUnwatched}.
+ */
+ private void logWalkFailure(Path dir, IOException e) {
+ Debug.logWarning("Hot-reload: could not fully walk " + dir + ": " +
e.getMessage(), MODULE);
+ }
+
+ /**
+ * Like {@link #registerAll}, but also dispatches every regular file
already present
+ * under {@code start} through {@link #dispatchChangedFile}, as if a
CREATE event had
+ * fired for each. Used only when a brand-new directory materializes
mid-session (see
+ * {@link #watchLoop}) -- deliberately not used for the bulk registration
done at
+ * startup or the periodic re-registration in {@link #applyCompile}, where
+ * re-dispatching every already-known file on every call would be both
wasteful and
+ * cause spurious repeat reloads.
+ */
+ private void registerAllAndSeed(Path start) throws IOException {
+ walkAndRegister(start, true);
+ }
+
+ private void walkAndRegister(Path start, boolean seedExistingFiles) throws
IOException {
Files.walkFileTree(start, new SimpleFileVisitor<Path>() {
@Override
public FileVisitResult preVisitDirectory(Path dir,
BasicFileAttributes attrs) {
- watchDirsAttempted++;
- try {
- dir.register(watchService,
- StandardWatchEventKinds.ENTRY_CREATE,
- StandardWatchEventKinds.ENTRY_MODIFY);
- } catch (IOException e) {
- warnUnwatched(dir, e);
- } catch (Throwable t) {
- // register() itself should only throw IOException, but
nothing here is
- // worth crashing the whole startup over.
- Debug.logError(t, "Hot-reload: unexpected error
registering watch for " + dir
- + " -- this directory will not be watched.",
MODULE);
+ registerWatch(dir);
+ return FileVisitResult.CONTINUE;
+ }
+
+ @Override
+ public FileVisitResult visitFile(Path file, BasicFileAttributes
attrs) {
+ if (seedExistingFiles) {
+ dispatchChangedFile(file.getParent(), file);
}
return FileVisitResult.CONTINUE;
}
});
}
+ /**
+ * Registers {@code dir} with the WatchService for create/modify events,
incrementing
+ * {@link #watchDirsAttempted} and calling {@link #warnUnwatched} on
failure instead of
+ * letting it propagate. Shared by {@link #registerServicedefDirs} and
+ * {@link #walkAndRegister} so both directory-registration paths get the
same specific
+ * failure handling instead of each hand-rolling its own copy.
+ *
+ * @return {@code true} if the registration succeeded.
+ */
+ private boolean registerWatch(Path dir) {
+ watchDirsAttempted++;
+ try {
+ dir.register(watchService, StandardWatchEventKinds.ENTRY_CREATE,
StandardWatchEventKinds.ENTRY_MODIFY);
+ return true;
+ } catch (IOException e) {
+ warnUnwatched(dir, e);
+ } catch (Throwable t) {
+ // register() itself should only throw IOException, but nothing
here is worth
+ // crashing the whole startup over.
+ Debug.logError(t, "Hot-reload: unexpected error registering watch
for " + dir
+ + " -- this directory will not be watched.", MODULE);
+ }
+ return false;
+ }
+
/**
* Records that {@code dir} could not get a WatchService registration
(most commonly
* the OS's per-process watch ceiling, e.g. macOS's kqueue-per-directory
cost) and
@@ -837,8 +895,7 @@ public class DevReloadContainer implements Container {
watchDirsFailed++;
Debug.logWarning("Hot-reload: could not watch " + dir + " (" +
cause.getMessage() + ") -- changes "
+ "there will not be picked up until OFBiz is restarted.
Narrow the watched set with "
- + "-Dofbiz.hotreload.components=compA,compB (or
-Photreload.components=compA,compB with the "
- + "ofbizDev Gradle task) to fit under the OS watch limit.",
MODULE);
+ + SCOPE_HINT, MODULE);
}
/**
diff --git
a/devreload/src/main/java/org/apache/ofbiz/devreload/RecordingFileManager.java
b/devreload/src/main/java/org/apache/ofbiz/devreload/RecordingFileManager.java
new file mode 100644
index 000000000..3fcb8e669
--- /dev/null
+++
b/devreload/src/main/java/org/apache/ofbiz/devreload/RecordingFileManager.java
@@ -0,0 +1,54 @@
+/*******************************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+
*******************************************************************************/
+package org.apache.ofbiz.devreload;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import javax.tools.FileObject;
+import javax.tools.ForwardingJavaFileManager;
+import javax.tools.JavaFileManager;
+import javax.tools.JavaFileObject;
+import javax.tools.StandardJavaFileManager;
+
+/**
+ * Wraps the compiler's standard file manager to record the absolute path of
every
+ * {@code .class} file it actually writes, so {@link DevReloadContainer}'s
post-compile
+ * reload step can hot-swap exactly what the compiler produced instead of
guessing from
+ * the source file's own base name -- a guess that misses a secondary
top-level class
+ * declared in the same file, or a lone non-public top-level class named
differently
+ * from its file.
+ */
+final class RecordingFileManager extends
ForwardingJavaFileManager<StandardJavaFileManager> {
+ final List<Path> outputFiles = new ArrayList<>();
+
+ RecordingFileManager(StandardJavaFileManager fileManager) {
+ super(fileManager);
+ }
+
+ @Override
+ public JavaFileObject getJavaFileForOutput(JavaFileManager.Location
location, String className,
+ JavaFileObject.Kind kind, FileObject sibling) throws IOException {
+ JavaFileObject file = super.getJavaFileForOutput(location, className,
kind, sibling);
+ outputFiles.add(Paths.get(file.toUri()));
+ return file;
+ }
+}
