Skip to content

Conversation

@JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Jun 5, 2025

Pull Request Description

Modifies #13206 to not need +H:ForeignAPISupport by alternatively using UnmanagedMemory and ByteBuffer on each side of the Channel.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • All code follows the
    Scala,
    Java,
  • Unit tests have been written where possible.

@JaroslavTulach JaroslavTulach self-assigned this Jun 5, 2025
@github-actions
Copy link

github-actions bot commented Jun 5, 2025

✨ GUI Checks Results

Summary

  • 🧹 Prettier: ✅ Passed
  • 🧰 GUI Checks: ❌ Failed
  • 📚 Storybook: ❌ Failed

⚠️ Action Required: Please review the failed checks and fix them before merging.

See individual check results for more details.

Copy link
Collaborator

@hubertp hubertp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we potentially detect the unsupported platform and leave the old support, if we feel that it looks/is less complex? It certainly feels less hacky than using ByteBuffers.

And when GraalVM 25.x lands, we could easily go back to the previous implementation.

@JaroslavTulach
Copy link
Member Author

Things seem to work on Mac Arm now:

sbt:enso> os-environment/test

passes.

@JaroslavTulach
Copy link
Member Author

Could we potentially detect the unsupported platform and leave the old support, if we feel that it looks/is less complex?

  • the good thing on the previous version was that MemorySegment was available in both SVM and HotSpot JVM mode
  • this is no longer true, only ByteBuffer is available on both JVMs
  • as such changes like 435d196 are necessary

It certainly feels less hacky than using ByteBuffers.

  • it is not that bad
  • one just has to remember to allocate native memory with UnmanagedMemory on the SVM side
  • otherwise MemorySegment is 1:1 with direct ByteBuffer

@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Jun 5, 2025
@JaroslavTulach JaroslavTulach changed the title Avoid the need for +H:ForeignAPISupport Avoid the need for -H:+ForeignAPISupport Jun 5, 2025
@JaroslavTulach JaroslavTulach merged commit 52b7165 into develop Jun 5, 2025
79 of 83 checks passed
@JaroslavTulach JaroslavTulach deleted the wip/jtulach/AvoidForeignAPIInSvm13206 branch June 5, 2025 16:13
@enso-bot enso-bot bot mentioned this pull request Jun 6, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI: No changelog needed Do not require a changelog entry for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants