-
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
Draft
Akirathan
wants to merge
32
commits into
develop
Choose a base branch
from
wip/akirathan/13816-bindings-via-arena
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
06ae858
Test that second cache serialization fails
Akirathan ab0fc5e
Cache uses native memory Arena
Akirathan 883dc14
Add optional CacheStatistics to EnsoContext
Akirathan 963afda
Use logName instead of entryName
Akirathan 5a14330
Test save of suggestions and bindings caches
Akirathan 24b7c4d
Use hamcrest matchers
Akirathan 58b92e8
Add test for loading caches
Akirathan 822bf5d
Rename test
Akirathan e3a1c36
Test that loading bindings cache of big project should be mmapped
Akirathan 54deef1
refact: extract method
Akirathan cef3885
Test that cache can be saved.
Akirathan 04eaf57
Test global cache save preference
Akirathan 225beaa
More cache tests
Akirathan 3e362b8
Use only primitive bytes
Akirathan f63d3a3
Revert "Use logName instead of entryName"
Akirathan be04961
Revert "Add optional CacheStatistics to EnsoContext"
Akirathan ae1d687
Test caches by collecting logs
Akirathan 4f01c2c
CachedData test class is an empty class.
Akirathan 5bce4bc
Add memory arena validity assert
Akirathan 46b42e5
First close memory arena and then writeBytesTo
Akirathan 8653350
Memory arena must be shared.
Akirathan 2756b69
Cache.invalidate closes memory arena
Akirathan 7cf44a9
Test that memory arena is closed after cache is saved
Akirathan a38623a
Test that byteBuffer from deserialization is read only
Akirathan 74a9fc6
readability is tested via ByteBuffer.get
Akirathan e79df91
Random is not static field
Akirathan 79ecc3c
Test that byteBuffer is closed after cache is invalidated
Akirathan 00e3786
randomBytes is not static method
Akirathan b1bd76a
fmt
Akirathan c7cbcac
Add `-H:+ForeignAPISupport` NI flag
Akirathan 72478c0
New Arena is created before mmap.
Akirathan 1de26ec
Merge branch 'develop' into wip/akirathan/13816-bindings-via-arena
Akirathan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
3 changes: 2 additions & 1 deletion
3
...e/runner/src/main/resources/META-INF/native-image/org/enso/runner/native-image.properties
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
Args=--features=org.enso.runner.EnsoLibraryFeature | ||
Args = --features=org.enso.runner.EnsoLibraryFeature \ | ||
JaroslavTulach marked this conversation as resolved.
Show resolved
Hide resolved
|
||
-H:+ForeignAPISupport |
267 changes: 267 additions & 0 deletions
267
engine/runtime-integration-tests/src/test/java/org/enso/interpreter/caches/CacheTests.java
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,267 @@ | ||
package org.enso.interpreter.caches; | ||
|
||
import static org.hamcrest.CoreMatchers.containsString; | ||
import static org.hamcrest.MatcherAssert.assertThat; | ||
import static org.hamcrest.Matchers.is; | ||
import static org.hamcrest.Matchers.notNullValue; | ||
import static org.junit.Assert.fail; | ||
|
||
import com.oracle.truffle.api.TruffleLogger; | ||
import java.io.IOException; | ||
import java.lang.foreign.Arena; | ||
import java.nio.ByteBuffer; | ||
import java.util.Optional; | ||
import java.util.Random; | ||
import java.util.concurrent.atomic.AtomicReference; | ||
import java.util.function.Supplier; | ||
import java.util.logging.Level; | ||
import org.enso.interpreter.caches.Cache.Roots; | ||
import org.enso.interpreter.caches.Cache.Spi; | ||
import org.enso.interpreter.runtime.EnsoContext; | ||
import org.enso.test.utils.ContextUtils; | ||
import org.junit.Rule; | ||
import org.junit.Test; | ||
import org.junit.rules.TemporaryFolder; | ||
|
||
public final class CacheTests { | ||
|
||
@Rule public final TemporaryFolder tempFolder = new TemporaryFolder(); | ||
@Rule public final ContextUtils ctx = ContextUtils.createDefault(); | ||
private final Random random = new Random(42); | ||
|
||
@Test | ||
public void cacheCanBeSaved_ToLocalCacheRoot() throws IOException { | ||
var cacheRoots = createCacheRoots(); | ||
var ensoCtx = ctx.ensoContext(); | ||
var spi = new CacheSpi(cacheRoots); | ||
var cache = Cache.create(spi, Level.FINE, "testCache", false, false); | ||
var ret = cache.save(new CachedData(), ensoCtx, false); | ||
assertThat("was saved to local cache root", ret, is(cacheRoots.localCacheRoot())); | ||
var localCacheFile = | ||
cacheRoots.localCacheRoot().resolve(CacheSpi.ENTRY_NAME + CacheSpi.DATA_SUFFIX); | ||
assertThat("local cache file was created", localCacheFile.exists(), is(true)); | ||
} | ||
|
||
@Test | ||
public void globalCacheIsPreferred() throws IOException { | ||
var cacheRoots = createCacheRoots(); | ||
var ensoCtx = ctx.ensoContext(); | ||
var spi = new CacheSpi(cacheRoots); | ||
var cache = Cache.create(spi, Level.FINE, "testCache", false, false); | ||
var ret = cache.save(new CachedData(), ensoCtx, true); | ||
assertThat("was saved to global cache root", ret, is(cacheRoots.globalCacheRoot())); | ||
var globalCacheFile = | ||
cacheRoots.globalCacheRoot().resolve(CacheSpi.ENTRY_NAME + CacheSpi.DATA_SUFFIX); | ||
assertThat("global cache file was created", globalCacheFile.exists(), is(true)); | ||
} | ||
|
||
@Test | ||
public void memoryArenaIsClosed_AfterCacheSave() throws IOException { | ||
var ensoCtx = ctx.ensoContext(); | ||
var cacheRoots = createCacheRoots(); | ||
var spi = new CacheSpi(cacheRoots); | ||
var bigData = randomBytes(10 * 1024 * 1024); | ||
saveToLocalRoot(bigData, cacheRoots); | ||
|
||
var memoryArena = new AtomicReference<Arena>(); | ||
Supplier<Arena> arenaSupplier = | ||
() -> { | ||
memoryArena.set(Arena.ofConfined()); | ||
return memoryArena.get(); | ||
}; | ||
var cache = Cache.create(spi, Level.FINE, "testCache", false, false, arenaSupplier); | ||
var loaded = cache.load(ensoCtx); | ||
assertThat("was loaded", loaded.isPresent(), is(true)); | ||
assertThat("New arena was created", memoryArena.get(), is(notNullValue())); | ||
|
||
var ret = cache.save(new CachedData(), ensoCtx, false); | ||
assertThat("was saved", ret, is(notNullValue())); | ||
assertThat( | ||
"Memory arena is closed after cache save", memoryArena.get().scope().isAlive(), is(false)); | ||
} | ||
|
||
@Test | ||
public void cacheCannotBeLoaded_WithoutMetadataOnDisk() throws IOException { | ||
var ensoCtx = ctx.ensoContext(); | ||
var cacheRoots = createCacheRoots(); | ||
var localCacheFile = | ||
cacheRoots.localCacheRoot().resolve(CacheSpi.ENTRY_NAME + CacheSpi.DATA_SUFFIX); | ||
// Saving only data and no metadata | ||
try (var os = localCacheFile.newOutputStream()) { | ||
os.write(new byte[] {42}); | ||
} | ||
var spi = new CacheSpi(cacheRoots); | ||
var cache = Cache.create(spi, Level.FINE, "testCache", false, false); | ||
var loaded = cache.load(ensoCtx); | ||
assertThat("Cannot be loaded without metadata on disk", loaded.isPresent(), is(false)); | ||
} | ||
|
||
@Test | ||
public void byteBufferIsClosed_AfterCacheIsInvalidated() throws IOException { | ||
var ensoCtx = ctx.ensoContext(); | ||
var cacheRoots = createCacheRoots(); | ||
var bigData = randomBytes(10 * 1024 * 1024); | ||
saveToLocalRoot(bigData, cacheRoots); | ||
var spi = new CacheSpi(cacheRoots); | ||
var cache = Cache.create(spi, Level.FINE, "testCache", false, false); | ||
var loaded = cache.load(ensoCtx); | ||
var deserializeBuffer = spi.deserializeBuffer; | ||
assertThat("was loaded", loaded.isPresent(), is(true)); | ||
|
||
cache.invalidate(ensoCtx); | ||
|
||
try { | ||
deserializeBuffer.get(); | ||
fail("Expected IllegalStateException - cannot read from byte buffer anymore"); | ||
} catch (IllegalStateException e) { | ||
assertThat(e.getMessage(), containsString("closed")); | ||
} | ||
} | ||
|
||
@Test | ||
public void byteBufferIsValid_AfterCacheLoad() throws IOException { | ||
var ensoCtx = ctx.ensoContext(); | ||
var cacheRoots = createCacheRoots(); | ||
var bigData = randomBytes(10 * 1024 * 1024); | ||
saveToLocalRoot(bigData, cacheRoots); | ||
var spi = new CacheSpi(cacheRoots); | ||
var cache = Cache.create(spi, Level.FINE, "testCache", false, false); | ||
var loaded = cache.load(ensoCtx); | ||
assertThat("was loaded", loaded.isPresent(), is(true)); | ||
|
||
var deserializeBuffer = spi.deserializeBuffer; | ||
var firstByte = deserializeBuffer.get(); | ||
assertThat("byte buffer is still readable", firstByte, is(bigData[0])); | ||
} | ||
|
||
@Test | ||
public void byteBufferIsReadonly() throws IOException { | ||
var ensoCtx = ctx.ensoContext(); | ||
var cacheRoots = createCacheRoots(); | ||
var bigData = randomBytes(10 * 1024 * 1024); | ||
saveToLocalRoot(bigData, cacheRoots); | ||
var spi = new CacheSpi(cacheRoots); | ||
var cache = Cache.create(spi, Level.FINE, "testCache", false, false); | ||
var loaded = cache.load(ensoCtx); | ||
assertThat("was loaded", loaded.isPresent(), is(true)); | ||
|
||
var deserializeBuffer = spi.deserializeBuffer; | ||
try { | ||
deserializeBuffer.put((byte) 42); | ||
fail("Expected ReadOnlyBufferException"); | ||
} catch (java.nio.ReadOnlyBufferException e) { | ||
// expected | ||
} | ||
} | ||
|
||
private Roots createCacheRoots() throws IOException { | ||
var cacheRootDirPath = tempFolder.newFolder("cacheRoot").toPath(); | ||
var localCacheDir = cacheRootDirPath.resolve("local"); | ||
var globalCacheDir = cacheRootDirPath.resolve("global"); | ||
localCacheDir.toFile().mkdir(); | ||
globalCacheDir.toFile().mkdir(); | ||
var ensoCtx = ctx.ensoContext(); | ||
return new Roots( | ||
ensoCtx.getTruffleFile(localCacheDir.toFile()), | ||
ensoCtx.getTruffleFile(globalCacheDir.toFile())); | ||
} | ||
|
||
/** Saves data as well as empty metadata on the disk. */ | ||
private static void saveToLocalRoot(byte[] data, Roots cacheRoots) throws IOException { | ||
var localCacheFile = | ||
cacheRoots.localCacheRoot().resolve(CacheSpi.ENTRY_NAME + CacheSpi.DATA_SUFFIX); | ||
var localMetadataFile = | ||
cacheRoots.localCacheRoot().resolve(CacheSpi.ENTRY_NAME + CacheSpi.METADATA_SUFFIX); | ||
try (var os = localCacheFile.newOutputStream()) { | ||
os.write(data); | ||
} | ||
try (var os = localMetadataFile.newOutputStream()) { | ||
os.write(42); | ||
} | ||
} | ||
|
||
private byte[] randomBytes(int size) { | ||
byte[] bytes = new byte[size]; | ||
random.nextBytes(bytes); | ||
return bytes; | ||
} | ||
|
||
private static final class CachedData {} | ||
|
||
private static final class Metadata {} | ||
|
||
private static final class CacheSpi implements Spi<CachedData, Metadata> { | ||
public static final String DATA_SUFFIX = ".test.data"; | ||
public static final String METADATA_SUFFIX = ".test.metadata"; | ||
public static final String ENTRY_NAME = "test-entry"; | ||
|
||
private final Roots cacheRoots; | ||
private ByteBuffer deserializeBuffer; | ||
|
||
private CacheSpi(Roots cacheRoots) { | ||
this.cacheRoots = cacheRoots; | ||
} | ||
|
||
@Override | ||
public CachedData deserialize( | ||
EnsoContext context, ByteBuffer data, Metadata meta, TruffleLogger logger) { | ||
deserializeBuffer = data; | ||
return new CachedData(); | ||
} | ||
|
||
@Override | ||
public byte[] serialize(EnsoContext context, CachedData entry) { | ||
return new byte[] {42}; | ||
} | ||
|
||
@Override | ||
public byte[] metadata(String sourceDigest, String blobDigest, CachedData entry) { | ||
return new byte[0]; | ||
} | ||
|
||
@Override | ||
public Metadata metadataFromBytes(byte[] bytes, TruffleLogger logger) { | ||
return new Metadata(); | ||
} | ||
|
||
@Override | ||
public Optional<String> computeDigest(CachedData entry, TruffleLogger logger) { | ||
return Optional.of("42"); | ||
} | ||
|
||
@Override | ||
public Optional<String> computeDigestFromSource(EnsoContext context, TruffleLogger logger) { | ||
throw new AssertionError("should not be called"); | ||
} | ||
|
||
@Override | ||
public Optional<Roots> getCacheRoots(EnsoContext context) { | ||
return Optional.of(cacheRoots); | ||
} | ||
|
||
@Override | ||
public String entryName() { | ||
return ENTRY_NAME; | ||
} | ||
|
||
@Override | ||
public String dataSuffix() { | ||
return DATA_SUFFIX; | ||
} | ||
|
||
@Override | ||
public String metadataSuffix() { | ||
return METADATA_SUFFIX; | ||
} | ||
|
||
@Override | ||
public String sourceHash(Metadata meta) { | ||
return "42"; | ||
} | ||
|
||
@Override | ||
public String blobHash(Metadata meta) { | ||
return "42"; | ||
} | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
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.
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.
runtime-caches
module:runtime
)/provides patternruntime-caches
module is missing, thenruntime
would just avoid loading cachesruntime
inside of IGV/VSCode on LTS JDK (e.g. JDK 21)runtime
in there anywayruntime-compiler
at maxruntime-compiler
can still run on JDK21, maybe even JDK17There 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