-
Notifications
You must be signed in to change notification settings - Fork 5
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
Improve IORef/Buffer implementations #4
Comments
One 'solution' could be to implement IORef as a NIF which stores the value that is put inside fully inside the Rust struct (using e.g. The main disadvantage over storing Erlang references is obviously that we need to copy datatypes when putting them in the IORef rather than being able to rely on Erlang's builtin reference counting here. However, it should not leak memory as the IORefs themselves are now GC'd correctly. |
Interestingly |
Another question: Why is it necessary to be able to resize the internals of an IORef? If this is not actually necessary you might be able to use |
Thanks for all your suggestions! 😃 I need to look more into them. For now, I will try to answer your questions with my current understanding. Data.IORef
To illustrate this problem in terms of Erlang. I am using Ref = atomics:new(1, []).
atomics:get(Ref, 1). The problem is that when the reference is serialised, there are no more references, and the SerialisedRef = term_to_binary(atomics:new(1, [])).
atomics:get(binary_to_term(SerialisedRef), 1). Results in an error:
Resizing IORefThe way Data.IORef works is that the value it contains can be changed at any later point. The size of the value that is stored in the IORef might vary wildly. There are no upper bound to how much data can be stored in the IORef, which means it needs to be resized if it is too small. import Data.IORef
main : IO ()
main = do
ref <- newIORef "small string"
smallStr <- readIORef ref
putStrLn smallStr
writeIORef ref "very long string. very long string"
longStr <- readIORef ref
putStrLn longStr Prints:
Both strings are written to the same IORef-reference. I was thinking that maybe it is possible to create a new Data.BufferA quick search in the Idris 2's source code indicate that the |
I see. For some reason I thought that it would always itself contain a reference but on hindsight that does not make any sense. That indeed is a clear reason why Also thank you for more information about I expect that the easiest way forward would then be to implement it as a NIF. As far as I can see, ResourceArc will not serialize what is to be stored inside, but instead increment the reference pointer of the thing it contains on construction and decrement it on destruction. |
Thanks again! If it is possible to avoid serializing the data from Erlang, that might work! My current implementation of IORef is built on top of Buffer, which means the data from Erlang is serialized. I added a small test file that reproduces the error (in branch ioref-test). My Rust skills is not really up to par. If you would like to give it a try, I would be very happy 😊 I had to go through some hoops in order for the |
Author of Rustler here, just dropping in to give some context/clarifications. What NIF resources do, is they allow you to opaquely store data inside a handle that is managed and garbage collected by the erlang VM. They pretty much only wrap a pointer, and do not allow storing terms inside by themselves. When using Interestingly, there is actually a way to own and store terms in native data structures in NIFs, and that's owned environments. However, the caveat here is that this requires copying terms into and out of that owned environment whenever you want to pass it to/from the process the NIF runs in. There is also no way to deallocate individual terms from the owned env without clearing the whole thing. If you simply need to serialize a terms as a binary, the most performant and simple way would be to use |
Thank you for the insights, @hansihe! ❤️ Also thanks for making Rustler! It was very easy to get it going, even for one that was completely new to Rust. |
By the way, IORef and Buffer are very low-level primitives which Idris2 uses for increased efficiency. |
@Qqwy That's true. I think the biggest reason for supporting IORef and Buffer is that they might be used in some Idris 2 libraries. Looking at the modules in the Idris 2 libraries (
Another reason is that IORef and Buffer are used in the Idris 2 compiler. The Idris 2 compiler is already running on the BEAM, but it would be even nicer if the Erlang version was close to the same performance as the Chez Scheme version, and that it did not leak memory. With that said, there might be other ways to achieve this: By rewriting the parts of the compiler that uses IORef and Buffer. Rewriting just these parts might not be sufficient though. In general, I would say that IORef and Buffer are not needed. When writing Idris 2 code that is intended for Erlang there are also other options, like using ETS, GenServer etc. |
Data.IORef
,Data.IOArray
(Built on top ofData.IORef
) andData.Buffer
are all abstractions that provide mutability. Implementing them in Erlang turns out to be not so easy (to me, at least).I recommend to not use the
Data.IORef
,Data.IOArray
andData.Buffer
modules in your code if you plan to generate Erlang, as these primitives currently leaks memory.NIF
I implemented both the IORef and Buffer types as a NIF: https://github.com/chrrasmussen/mutable_storage [Rust]
The Buffer implementation works as expected, and IORef implementation mostly works. The blocker is that IORef can't store Erlang references, or rather, if it does, the Erlang reference gets serialised and value pointed to by the reference is potentially garbage collected. When reading back the Erlang reference it might not point to anything, which would lead to run-time error if accessed.
Using NIF also makes the project harder to distribute.
Possible solutions
It might be possible to solve the issue in the Idris 2 compiler (or the code generator)
Or a long shot:
IORef
)IORef
,IOArray
andBuffer
)Current implementation
The current implementations of
Data.IORef
andData.Buffer
are using process dictionary (ETS would probably work as well), which means they leak memory.The text was updated successfully, but these errors were encountered: