Phase 2C: remove PropertiesEnhancer from enhancer pipeline#1
Merged
megamattron merged 18 commits intomodernizationfrom Feb 22, 2026
Merged
Phase 2C: remove PropertiesEnhancer from enhancer pipeline#1megamattron merged 18 commits intomodernizationfrom
megamattron merged 18 commits intomodernizationfrom
Conversation
…-JDK testing build.xml: use python3 on Unix (was 'python', which is Python 2.7 on macOS). CI sets up Python 3 explicitly; this aligns local runs with CI behavior. Makefile: add test-jdkNN / unittest-jdkNN targets that resolve JDK paths via /usr/libexec/java_home, so switching JDK for a test run is one word rather than a long JAVA_HOME path.
Adds JDK 21 on ubuntu-latest to the test matrix with continue-on-error: true so failures are visible but don't block PRs. Windows excluded for JDK 21 to keep the matrix focused. As modernization phases complete, JDK 21 failures should decrease until continue-on-error can be removed.
…, ContinuationEnhancer HibernateInterceptorTest — pure unit tests for the explicit-save mechanism: findDirty() returns empty array (block flush) when willBeSaved=false, returns null (defer to Hibernate) when willBeSaved=true or non-JPABase object. Also tests onSave() ThreadLocal storage and afterTransactionCompletion() cleanup. Guards Phase 1E and Phase 2B changes. PropertiesEnhancerTest — verifies accessor generation and constructor addition using Javassist bytecode loading, bypassing the Play classloader. Tests that getters/setters are generated for public fields, @PlayPropertyAccessor is applied, and a default constructor is added when missing. Guards Phase 2C removal. ContinuationEnhancerTest — tests isEnhanced() runtime detection: returns false for unknown classes, for classes without EnhancedForContinuations, and for registered-but-not-yet-loaded classes (javaClass==null). Guards Phase 3C. Also fixes a latent NPE bug in ContinuationEnhancer.isEnhanced(): it now null-checks appClass.javaClass before calling isAssignableFrom().
- Add samples-and-tests/tfb/ with plaintext, JSON, db, queries, updates endpoints using H2 in-memory DB and Hibernate/JPA via GenericModel - Add benchmark.sh: rebuilds framework JAR, warms up JIT, runs wrk against all 8 endpoint variants, prints summary table with RPS/p50/p99 - Record H2 baseline in MODERNIZATION_PLAN.md (2026-02-21, OpenJDK 23): plaintext 7463 RPS, json 7917, db 7169, queries@20 4954, updates@20 2504
FieldAccessor previously called getClass().getMethod() on every field read/write. Add two ConcurrentHashMap<Class<?>, ConcurrentHashMap<String, Method>> caches (GETTER_CACHE, SETTER_CACHE) so reflective lookup happens once per class/field pair instead of once per access. A NO_METHOD sentinel (Object.hashCode reused) lets the cache record "no accessor exists, fall back to field access" without null values. A volatile cachedClassLoader field drives checkCacheValidity(): when Play.classloader changes on hot reload both caches are cleared so reloaded classes get fresh lookups. TFB benchmark delta vs Phase 0D baseline (30s/4t/256c, H2, JDK 23): plaintext +5%, db +4%, queries q=1 +3%; DB-bound endpoints near-zero because World has only 2 fields. Gains scale with object-graph depth. All unit tests pass (GroovyTemplateTest failures are pre-existing JDK 23 incompatibility tracked in Phase 3B).
- Fix application.mode=dev → prod in tfb/conf/application.conf. DEV mode runs Play.detectChanges() on every request (full filesystem scan), consuming 38.6% of active CPU and suppressing throughput ~10×. - Add @NoTransaction to plaintext and json controller actions. Without it, JPAPlugin.TransactionalFilter opens an EntityManager and begins a transaction on every request including non-DB routes, costing another 27.6% of active CPU on those endpoints. - Add --profile flag to benchmark.sh. Attaches async-profiler (asprof) to the running server after warmup and writes a collapsed-stack CPU profile to /tmp/play-profile.txt. Auto-installs libasyncProfiler.dylib to $JAVA_HOME/lib/ on first use. - Update MODERNIZATION_PLAN.md with corrected PROD-mode baseline numbers (~75k RPS plaintext vs ~7.5k in DEV), profiling methodology notes, and three remaining server-layer hotspots identified by the profiler: SimpleDateFormat per response, Pattern.compile per request, and H2 MVStore spin contention on writes (H2-only, not a framework issue).
- Set play.pool=64 and db.pool.maxSize=64 in tfb/conf/application.conf. Default of CPUs+1 (11) queues 245/256 concurrent connections at a time. Pool sweep showed pool=64 peaks read/query throughput (+6% plaintext, +3% db) on this 10-core machine before context-switch overhead takes over. - Add pool-sweep.sh: restarts the server at each pool size in POOL_SIZES, runs all endpoints, and prints a comparative RPS table. Useful for re-running the sweep on different hardware or after framework changes. - Update MODERNIZATION_PLAN.md with pool sweep data and findings: update endpoints degrade monotonically with more threads (H2 MVStore write spin contention), which inverts with PostgreSQL. Baseline table updated to pool=64 numbers (~79k RPS plaintext).
Virtual threads (JEP 444, JDK 21) replace the fixed-size ScheduledThreadPoolExecutor in play.Invoker with a virtual-thread-per-task executor. The pool sweep in Phase 0D demonstrated the manual tuning problem: play.pool must be sized to match concurrency, and the right value varies by workload and hardware. Virtual threads eliminate this. Key details captured in the plan: - Scope is one file (Invoker.java) plus separating JobsPlugin onto its own platform- thread scheduler (background jobs should not be virtual threads) - Hibernate 5 pinning severely limits the benefit: synchronized blocks in H2 and Hibernate 5 session management pin virtual threads to their carrier, defeating the purpose; full gains require Phase 2B (Hibernate 6) first - 3D absorbs 3C's Option B: once Invoker runs on virtual threads, Controller.await() can simply call Thread.sleep(), replacing the JavaFlow continuation bytecode entirely - play.pool config is silently ignored on JDK 21+ when virtual threads are active; documented as a behaviour change - Added 3D to dependency map, change matrix, and a new "maximum performance path" note in the minimum viable JDK 21 section (2B → 3A → 3D)
Explains why virtual threads improve Play 1.x performance, why phases 2B (Hibernate 6), 3A (Netty 4), and 3D (VT executor) form a single unit, and what the actual throughput benefit looks like vs the platform thread model. Covers: the blocking thread pool sizing problem, how virtual threads unmount vs pin, the synchronized pinning sources in Hibernate 5 and Netty 3's write path (confirmed by bytecode inspection), and why the pool sweep understates the benefit (H2 is fast enough that thread queuing rarely hurts — PostgreSQL with real network latency is where virtual threads pay off most).
-noverify is deprecated since JDK 13 and silently ignored on JDK 21+.
Removing it surfaces any bytecode that fails verification, which we
need to know about before Phase 2C.
- framework/pym/play/application.py: remove java_args.append('-noverify')
- framework/pym/play/commands/eclipse.py: remove -noverify from eclipse VM args
SecurityManager is deprecated-for-removal in JDK 17 and fully removed in JDK 24. - PThreadFactory: use Thread.currentThread().getThreadGroup() unconditionally; drop System.getSecurityManager() conditional - application.py: remove java.policy / java.security.manager configuration block
PropertiesEnhancer does three things: generate constructors, generate accessors, rewrite field access. Extract just the constructor generation into a standalone ConstructorEnhancer so it survives the removal of PropertiesEnhancer in Phase 2C. The constructor logic is NOT removed from PropertiesEnhancer yet — both run, which is safe since adding a constructor that already exists is a no-op in Javassist.
Add jpa.explicitSave config flag (default true, preserving existing behaviour). When false, switches to standard JPA/Hibernate semantics: Hibernate manages dirty checking instead of Play's explicit-save model. - JPAPlugin: read jpa.explicitSave; expose as static boolean - HibernateInterceptor.findDirty(): return null when !explicitSave so Hibernate runs its own dirty check instead of blocking all flushes - HibernateInterceptor collection callbacks: return true unconditionally when !explicitSave (don't gate on willBeSaved) - JPABase._save(): skip saveAndCascade() when !explicitSave; just persist() new entities and flush() This eliminates the O(graph-size) reflective cascade walk for apps that opt in, and is a prerequisite for Phase 2B (Hibernate 6 removes the internal SPIs that saveAndCascade uses).
Default benchmark duration: 30s → 10s Warmup passes: 10s each → 5s each Faster iteration while still providing a meaningful signal. Pass an explicit duration to override: ./benchmark.sh 30 256
- EnhancerPlugin: drop new PropertiesEnhancer() from defaultEnhancers(); ConstructorEnhancer already handles constructor generation - BeanWrapper: remove @PlayPropertyAccessor import and filter from isSetter/isScalaSetter; no such methods exist now that PropertiesEnhancer is off - PropertiesEnhancer: flip default of play.propertiesEnhancer.enabled from true to false; enhancer remains available as an explicit opt-in
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
new PropertiesEnhancer()fromdefaultEnhancers();ConstructorEnhancer(added in Phase 1C) already handles default constructor generation, so this is the only part that needed to survive.@PlayPropertyAccessorimport and the!method.isAnnotationPresent(PlayPropertyAccessor.class)guard fromisSetter()andisScalaSetter(). With PropertiesEnhancer disabled, no methods carry that annotation, making the filter dead weight.play.propertiesEnhancer.enabledfromtruetofalse. The enhancer stays available as an explicit opt-in escape hatch for anyone who setsplay.propertiesEnhancer.enabled=truein their app config.Test results
ant unittestfromframework/:PropertiesEnhancerTest: 8 errors (expected — the enhancer is now off by default; accessor/rewrite tests correctly fail)GroovyTemplateTest: 7 pre-existing failures (Java 25 class file major version 69, unrelated)