Repository navigation
Conversation
- SpecsIo.createTempDirectory(prefix): unique directory allocated by the OS under the user temp folder, registered for deletion on exit as a backstop, so callers no longer hand-roll UUID/timestamp names - SpecsIo.getOsCacheFolder(): user-level cache folder following OS conventions (XDG_CACHE_HOME, ~/Library/Caches, %LOCALAPPDATA%) - SpecsSystem.isMac()
Add a backed store that decodes individual fields on first access, preserves deferred copies, and lets typed writes replace pending values. Verify storage semantics and run the full jOptions suite and coverage gate.
Build definition key maps before publication and use a concurrent cache for slot indexes. Parallel first readers previously observed incomplete maps, causing lazy AST imports to reject valid fields. Add a concurrent first-use test that reproduces the old failure.
Remove the FlatBuffers lazy datastore and slot resolution hooks from the eager production branch. Preserve the concurrency regression using ordinary stores; the complete lazy implementation remains on lazy-flatbuffers-experiment. Validation: jOptions tests and JaCoCo coverage gate pass.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff2b543328
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.appName = appName; | ||
| this.sections = new ArrayList<>(sections); | ||
| this.defaultData = defaultData; | ||
| this.keyMap = new HashMap<>(StoreDefinition.super.getKeyMap()); |
There was a problem hiding this comment.
StoreDefinition.super.getKeyMap() calls getKeys() virtually. If a downstream AStoreDefinition subclass overrides getKeys() and reads fields initialized after super(...), construction now sees uninitialized state and can throw or permanently cache an incomplete map; previously the lazy lookup occurred only after construction. Populate the map directly from the initialized sections or through a non-overridable base method.
Useful? React with 👍 / 👎.
| File tempFolder = Files.createTempDirectory(getTempFolder().toPath(), prefix).toFile(); | ||
|
|
||
| // Backstop deletion, in case the owner does not get the chance to delete it | ||
| deleteOnExit(tempFolder); |
There was a problem hiding this comment.
When callers put files into the returned directory, this registration does not provide the documented shutdown backstop: deleteOnExit traverses the directory immediately while it is still empty, so subsequently created children are never registered and deletion of the now-nonempty directory fails at JVM exit. Use cleanup that traverses at shutdown, or otherwise ensure later contents are removed before deleting the directory.
Useful? React with 👍 / 👎.
Return the immutable key copy configured with the supplier so generic and typed-list keys retain their defaults. Add regressions for supplier invocation, independent mutable list defaults in simple and closed stores, and typed-list setter validation.
lm-sousa
changed the title
Combine ead48b7's safe publication and atomic index cache with 19c8e3e's direct section traversal. Keep a concrete LinkedHashMap: the Protobuf unmodifiable wrapper breaks existing XStream XML persistence. Retain the stronger concurrency regression test and cover key ordering and subclass initialization. Full jOptions and SpecsUtils checks and coverage pass.
Use the Protobuf metadata snapshot while retaining FlatBuffers' atomic index cache and stronger concurrency regression. Cover immutable publication explicitly. Existing XStream persistence is incompatible: the final jOptions run passes 1725 tests and fails 17 legacy XML tests; this accepted incompatibility is recorded in the PR description pending separate XStream removal.
1 participant
Build shared datastore key maps completely before publication and cache slot indexes atomically. The final implementation uses Protobuf's direct section traversal and immutable ordered key map, avoiding calls to overridable accessors during construction; it retains FlatBuffers' stronger concurrent first-use test. Generic key factories preserve configured suppliers and fresh typed defaults.
getUsedMemory(callGc)now requests GC only when asked, removing the unconditional extra request. A JFR regression verifies zero requests for false and one for true. Clava #267 uses a concurrent per-class reader cache to avoid the shared metadata lookup on every node.Production stores remain eager; deferred experiments are preserved on
lazy-flatbuffers-experiment. Stacked on #34 and used by Clava #267.Compatibility warning: the immutable map breaks existing XStream XML persistence. XStream is being removed in a separate PR; until that lands, existing XML persistence and its tests remain incompatible. This PR does not claim a green jOptions check.
Validation at
ae7194a7b3e981913d40a4403acfc759808b6c3a: jOptions ran 1,742 tests, with 1,725 passing and 17 legacy XML failures (13 DataStoreXmlTest, four XmlPersistenceTest). The concurrent-publication, constructor, ordering, immutability and generic-default regressions passed. SpecsUtils check and JaCoCo verification passed: 6,654 tests passed and two skipped, including the GC-request regression. The dependent parser check passed 234 tests with four local-selector/disabled-cache skips, coverage and generated-output drift verification. Remote CI is reported separately.The dependent matched Clava benchmark completed 27 accepted rotated serial observations using this dependency revision. The combined ports lowered production Java test time 12.0%, while App construction improved under 1% in Java and JS. Updated FlatBuffers remains slower than Protobuf. This does not isolate this dependency patch's individual contribution, and does not remeasure memory.