Provide fully templated encoders and decoders #5

brodeuralexis · 2025-06-05T02:33:58Z

This PR provides a templated allocator for STL containers and a memory resource for std::pmr::polymorphic_allocator.

This enables the use of enif_alloc and enif_free from all STL containers while leaving the user the choice to use templates or polymorphism to select an allocator.

EDIT: Following the discussion below, the scope of this PR has been restrained to fully templated encoders and decoders. If a user wants a stateless allocator using enif_alloc and enif_free they can create one themselves.

README.md

jonatanklosko · 2025-06-06T14:33:29Z

Hey @brodeuralexis! This is an interesting idea, but I am not sure if it actually has the desired benefits. In practice C++ NIFs are almost always used to integrate a third-party C/C++ library, where a lot of allocations happen internally and is rather not configurable.

On a separate note, there is also a usability tradeoff. If we introduce fine::std_string and similar, we are diverging from "usual" C++ code, and the whole point of fine is to make the NIFs look more like basic C++. It's not necessarily a killer argument, but it is the reason why I am weary of extra user-side complexity.

brodeuralexis · 2025-06-06T15:02:55Z

To be honest, I hold no particular opinion on whether this should be included or not.

I mainly submitted this work because I am writing my libbpf wrapper with fine, and found those features could prove useful.

In practice C++ NIFs are almost always used to integrate a third-party C/C++ library, where a lot of allocations happen internally and is rather not configurable.

True, and most do it because it is a pain in the ass, and requires all the code to be in header files. To my knowledge, that is also why std::pmr exists, although it comes at the cost of an additional pointer per container to store the memory resource.

If we introduce fine::std_string and similar, we are diverging from "usual" C++ code, and the whole point of fine is to make the NIFs look more like basic C++.

I created those typedefs because I found typing std::basic_string<char, std::char_traits<char>, fine::Allocator<char>> long and tedious. We can always remove them, forcing the user to be extra explicit. This would also ensure that the user actually knows what they are doing with allocators.

Food for though

We can remove fine::memory_resource and fine::Allocator, but we should leave the templated allocators in the encoders and decoders so that users have the choice to implement their own fine::Allocator.
We can extract everything in a new header file (i.e.: fine/allocator.hpp) signifying it is an extension to existing functionalities.

jonatanklosko · 2025-06-08T12:45:45Z

I would say it's ok to start with 1. and upstream more changes if it comes up again, but looking at the decoders again, I've realised that we need a memory_resource during decoding, in order to allocate a new PMR container, right? And to make it configurable, we would need a separate version of decode, which I think is definitely too much :<

brodeuralexis · 2025-06-08T17:56:43Z

I'll rework the code to go with 1.

test/c_src/finest.cpp

jonatanklosko · 2025-06-11T11:46:15Z

README.md

+
+std::pmr::memory_resource* my_memory_resource = ...;
+
+std::vector<std::pmr::string, MyAllocator<std::pmr::string>> duplicate(


Should the return also be std::pmr::vector?

I would also be fine not having this section, because it might be one of the things where people who would think about using allocators would already know about those things. In this PR we just generalise the templates, so it's closer to how they would use the STL anyway?

No, I wanted a mix of stateless (std::vector) and stateful allocations (std::pmr::string) to ensure that both can be encoded correctly.

Right, the thing is that it's a usage example, and I would expect actual usage to consistently use either allocator type, not both. In other words, I wouldn't show code that the user wouldn't actually ever write. Again, I may be wrong!

[...] I wouldn't show code that the user wouldn't actually ever write.

Also links to #5 (comment).

I will make the change.

Wait, so should we use the same allocator for both then?

With my latest changes, only struct Allocator is left, and is used for testing both encoding and decoding.

Yes! What I mean is that the example still shows a mix of both allocators (std::vector and std::pmr::string), which is probably not something they would do in practice?

In other words, I would mention stateless allocators and make the example use std::vector and std::string, both with MyAllocator. Then, below, I would have the note about about polymorphic_allocator and get_default_resource.

Does that make sense?

README.md

This commits provides a templated allocator for STL containers and a memory resource for `std::pmr::polymorphic_allocator`. This enables the use of `enif_alloc` and `enif_free` from all STL containers while leave the user the choice to use templates or polymorphism to select an allocator.

Now, when decoding `std::pmr::*` STL containers, `fine::memory_resource` will be used instead of the default `std::pmr::get_default_resource()` memory resource.

The `operator[]` on `std::map` will default construct the value if it does not exist, and then move assign it the requested value. By using the C++17 method `insert_or_assign`, we prevent that extra object construction.

fine::Allocator and fine::memory_resource were deemed too complicated for use by users. On the flip side, we keep the templated allocators during encoding and decoding, ensuring that users can implemented their own stateless allocator for decoding and use any possible allocator for encoding.

Information about allocators is remove from the Encoding/Decoding section of the README.md. A column indicating the Elixir type associated with the C++ type has been added.

Co-authored-by: Jonatan Kłosko <[email protected]>

jonatanklosko · 2025-06-16T20:07:09Z

test/c_src/finest.cpp

@@ -1,5 +1,6 @@
 #include <cstring>
 #include <exception>
+#include <memory_resource>


Suggested change

#include <memory_resource>

jonatanklosko · 2025-06-16T20:14:35Z

test/test/finest_test.exs

+    test "allocators" do
+      assert NIF.allocators("abc", 16) ==
+               ["abc"] |> Stream.cycle() |> Stream.take(16) |> Enum.to_list()


Btw. I would add it to this test:

fine/test/test/finest_test.exs

Lines 145 to 146 in 66bd25f

test "vector" do

assert NIF.codec_vector_int64([1, 2, 3]) == [1, 2, 3]

something like this:

assert NIF.codec_vector_int64_alloc([1, 2, 3]) == [1, 2, 3]

Since we focus on encoding/decoding working as expected :)

brodeuralexis commented Jun 5, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

brodeuralexis force-pushed the feature/allocator branch 3 times, most recently from 96a8e8a to fece407 Compare June 6, 2025 14:22

brodeuralexis force-pushed the feature/allocator branch from a14bea3 to 30ba27a Compare June 8, 2025 18:21

brodeuralexis changed the title ~~Provide templated and polymorphic allocators~~ Provide fully templated encoders and decoders Jun 11, 2025

jonatanklosko reviewed Jun 11, 2025

View reviewed changes

brodeuralexis and others added 9 commits June 12, 2025 06:55

Ensure that std::pmr::* uses fine::memory_resource

6a724c3

Now, when decoding `std::pmr::*` STL containers, `fine::memory_resource` will be used instead of the default `std::pmr::get_default_resource()` memory resource.

Prevent extraneous default construction when decoding maps

6c996d0

The `operator[]` on `std::map` will default construct the value if it does not exist, and then move assign it the requested value. By using the C++17 method `insert_or_assign`, we prevent that extra object construction.

Remove some cognitive load

5ced683

Information about allocators is remove from the Encoding/Decoding section of the README.md. A column indicating the Elixir type associated with the C++ type has been added.

Document std::string* as binary

1ec4013

Co-authored-by: Jonatan Kłosko <[email protected]>

Make Elixir types more Elixir-ish

aaf375c

Co-authored-by: Jonatan Kłosko <[email protected]>

Make Elixir Type the last column

92142ef

Better documentation

3fd4d26

brodeuralexis force-pushed the feature/allocator branch from f86170d to 3fd4d26 Compare June 12, 2025 11:03

Simply unit test

66bd25f

jonatanklosko reviewed Jun 16, 2025

View reviewed changes


		std::pmr::memory_resource* my_memory_resource = ...;

		std::vector<std::pmr::string, MyAllocator<std::pmr::string>> duplicate(

	test "vector" do
	assert NIF.codec_vector_int64([1, 2, 3]) == [1, 2, 3]

Provide fully templated encoders and decoders #5

Are you sure you want to change the base?

Provide fully templated encoders and decoders #5

Uh oh!

Conversation

brodeuralexis commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jonatanklosko commented Jun 6, 2025

Uh oh!

brodeuralexis commented Jun 6, 2025

Food for though

Uh oh!

jonatanklosko commented Jun 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brodeuralexis commented Jun 8, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brodeuralexis commented Jun 5, 2025 •

edited

Loading

jonatanklosko commented Jun 8, 2025 •

edited

Loading