Question about critical native function behavior with for-loop variables

[Maurizio Cimadamore]

You beat me to it :-)  reproduced on my machine as well.

The C string to be passed to open needs to be written using 
MemorySegment::setString (which will copy the string bytes and append 
the required terminator at the end).

Maurizio

On 06/02/2025 12:50, Jorn Vernee wrote:
>
> Hey David,
>
> It looks like you're not passing a null-terminated string to open. 
> path.getBytes will not return a null-terminated string. It probably 
> works when using the stableBuffer because the paths you're using are 
> shorter then the buffer, and you fill the buffer with zeros every 
> iteration. So, the string in there will always be null-terminated.
>
> Jorn
>
> On 6-2-2025 13:17, David wrote:
>> Hi,
>>
>> I have a question about the behavior of critical functions inside a 
>> for-loop. I marked open() as critical (I know this is not an empty 
>> function like the java docs tells me i should use critical for, but I 
>> really wanted to try it). Wanting to see if it speeds things up. What 
>> I didn't expect was that it doesn't work well with variables created 
>> inside the loop itself, or at least that seems to be the case. Open() 
>> fails returning -1. To work around this issue I created a 
>> "stableBuffer" just outside the loop, which makes the code work all 
>> of the time.
>>
>> I just have two questions. Is this expected behavior for critical 
>> functions? Why does the stable buffer approach work consistently 
>> while using variables inside the loop fail after a few iterations?
>>
>> The loop that causes issues:
>> @Test void test() { var paths = filesTooRead; byte[] stableBuffer = 
>> new byte[4096]; for (var path : paths) { byte[] pathBytes = 
>> path.getBytes(); Arrays.fill(stableBuffer, (byte) 0); 
>> System.arraycopy(pathBytes, 0, stableBuffer, 0, pathBytes.length); // 
>> works all the time int fd = openFile(stableBuffer, 0, 0); // works 
>> for a couple of iterations// int fd = openFile(path.getBytes(), 0, 0); // int fd = 
>> openFile("/media/david/Data2/text_files/file_2299.bin".getBytes(), 0, 
>> 0); MemorySegment buffer = Arena.ofAuto().allocate(4); read(fd, 
>> buffer, 4); buffer.set(ValueLayout.JAVA_BYTE, 3, (byte) 0); // 
>> System.out.println(path + "content " + buffer.getString(0)); if (fd < 
>> 0) { System.err.println("Failed to open file:"); } closeFile(fd); } }
>> OS: Pop!_OS 22.04 LTS
>> Java version: JDK 24 EA Build 35 (2025/2/4)
>> Thank you for your time and feedback.
>>
>> Kind regards,
>> David Vlijmincx
>>
>> The entire class:
>> package bench; import org.junit.jupiter.api.Test; import 
>> java.io.IOException; import java.lang.foreign.*; import 
>> java.lang.invoke.MethodHandle; import java.nio.file.Files; import 
>> java.nio.file.Path; import java.util.Arrays; import 
>> java.util.stream.Stream; import static 
>> java.lang.foreign.ValueLayout.*; public class CriticalLoopTest { 
>> public static final String BENCHMARK_FILE_EXTENSION = ".bin"; public 
>> static final Path BASE_BENCHMARK_FILES_DIR = 
>> Path.of("/media/david/Data2/text_files"); public static final 
>> String[] filesTooRead; private static final MethodHandle open; 
>> private static final MethodHandle close; private static final 
>> MethodHandle read; static { Linker linker = Linker.nativeLinker(); 
>> open = linker.downcallHandle( 
>> linker.defaultLookup().find("open").orElseThrow(), 
>> FunctionDescriptor.of(JAVA_INT, ADDRESS, JAVA_INT, JAVA_INT), 
>> Linker.Option.critical(true) ); read = linker.downcallHandle( 
>> linker.defaultLookup().find("read").orElseThrow(), 
>> FunctionDescriptor.of(JAVA_INT, JAVA_INT, ADDRESS, JAVA_INT) ); close 
>> = linker.downcallHandle( 
>> linker.defaultLookup().find("close").orElseThrow(), 
>> FunctionDescriptor.ofVoid(JAVA_INT) ); try (Stream<Path> files = 
>> Files.walk(BASE_BENCHMARK_FILES_DIR)){ filesTooRead = files .filter(p 
>> -> p.getFileName().toString().endsWith(BENCHMARK_FILE_EXTENSION)) 
>> .map(Path::toString) .toArray(String[]::new); } catch (IOException e) 
>> { throw new RuntimeException(e); } } @Test void test() { var paths = 
>> filesTooRead; byte[] stableBuffer = new byte[4096]; for (var path : 
>> paths) { byte[] pathBytes = path.getBytes(); 
>> Arrays.fill(stableBuffer, (byte) 0); System.arraycopy(pathBytes, 0, 
>> stableBuffer, 0, pathBytes.length); // works all the time int fd = 
>> openFile(stableBuffer, 0, 0); // works sometimes // int fd = 
>> openFile(path.getBytes(), 0, 0); // int fd = 
>> openFile("/media/david/Data2/text_files/file_2299.bin".getBytes(), 0, 
>> 0); MemorySegment buffer = Arena.ofAuto().allocate(4); read(fd, 
>> buffer, 4); buffer.set(ValueLayout.JAVA_BYTE, 3, (byte) 0); // 
>> System.out.println(path + "content " + buffer.getString(0)); if (fd < 
>> 0) { System.err.println("Failed to open file:"); } closeFile(fd); } } 
>> public int read(int fd, MemorySegment mem, int len) { try { return 
>> (int) read.invokeExact(fd, mem, len); } catch (Throwable e) { throw 
>> new RuntimeException(e); } } public int openFile(byte[] filePath, int 
>> flags, int mode) { try { int fd = (int) 
>> open.invokeExact(MemorySegment.ofArray(filePath), flags, mode); if 
>> (fd < 0) { throw new RuntimeException("Failed to open file fd=" + 
>> fd); } return fd; } catch (Throwable e) { throw new 
>> RuntimeException(e); } } public static void closeFile(int fd) { try { 
>> close.invokeExact(fd); } catch (Throwable e) { throw new 
>> RuntimeException("Could not close file with FD:" + fd, e); } } }
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/panama-dev/attachments/20250206/70fb9054/attachment.htm>