-
Notifications
You must be signed in to change notification settings - Fork 334
Use foreign memory arena for big bindings cache #13872
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
engine/runtime/src/main/java/org/enso/interpreter/caches/CacheStatistics.java
Outdated
Show resolved
Hide resolved
.settings( | ||
frgaalJavaCompilerSetting, | ||
// Needed for `java.lang.Foreign`. | ||
customFrgaalJavaCompilerSettings("24"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I was afraid that this would be needed.
- I wanted to suggest to create a separate
runtime-caches
module:- move all the cache related code there
- invoke it with requires (from
runtime
)/provides pattern - if the
runtime-caches
module is missing, thenruntime
would just avoid loading caches
- Because such a change would prevent usage of
runtime
inside of IGV/VSCode on LTS JDK (e.g. JDK 21) - but we don't need
runtime
in there anyway- the plans we have count with usage of
runtime-compiler
at max - and
runtime-compiler
can still run on JDK21, maybe even JDK17
- the plans we have count with usage of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tracked in #13898
engine/runtime/src/main/java/org/enso/interpreter/caches/Cache.java
Outdated
Show resolved
Hide resolved
engine/runtime/src/main/java/org/enso/interpreter/caches/CacheEvent.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- close mmapped files first, only then write
- avoid yet another level of indirection - e.g. remove statistics abstractions
engine/runner/src/main/resources/META-INF/native-image/org/enso/runner/native-image.properties
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest to keep allocate new Arena
when mmapping.
engine/runtime/src/main/java/org/enso/interpreter/caches/Cache.java
Outdated
Show resolved
Hide resolved
engine/runtime/src/main/java/org/enso/interpreter/caches/Cache.java
Outdated
Show resolved
Hide resolved
Previous arena is closed both in load and save.
# Conflicts: # engine/runtime/src/main/java/org/enso/interpreter/caches/Cache.java
Closes #13816
Blocked by #13918
Pull Request Description
Using
java.lang.foreign.Arena
for mmapping a big cache file when loading the cache.Important Notes
load
andsave
closes previous arena.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.