Add some list-related NIFs #1795

mat-hek · 2025-08-12T13:58:17Z

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

bettio

First question here: did you notice a relevant performance increase by replacing these functions with a native implementation?
Can you leave some information about this here (and maybe it might be a good idea to add this to the commit message, so we can have some further context about why this change has been done).

One style comment: let's use shorter commit subject / PR title (as encouraged in general with git).

mat-hek · 2025-08-13T09:12:14Z

Hi @bettio, generally our goal is to make AtomVM more compatible with OTP, and these functions are extensively used in OTP, but implemented in the BEAM as NIFs. Thus, I think it's worthwhile to have them as NIFs regardless of whether the performance improvement is significant. However, I did a simple benchmark with lists:member:

      haystack = Enum.to_list(1..200_000) ++ [:needle]

      t = :erlang.system_time(:millisecond)
      true = :lists.member(:needle, haystack)
      true = :lists.member(:needle, haystack)
      true = :lists.member(:needle, haystack)
      true = :lists.member(:needle, haystack)
      true = :lists.member(:needle, haystack)
      :erlang.display(:erlang.system_time(:millisecond) - t)
      :ok

and on my machine the benchmarked part takes around 40 ms before these changes, and 25 ms after, so it's quite big improvement IMO

pguyot · 2025-08-17T21:23:59Z

Indeed BEAM has at least lists:member/2 and lists:keymember/3 as NIFs as OTP28.

For what it's worth, I ran the benchmark on jit branch: 52ms with JIT and 97ms without. The longest part definitely was generating the haystack.

mat-hek · 2025-08-18T08:25:50Z

@pguyot I wasn't aware you're working on JIT, that's great news! Generating the haystack takes very long indeed, not sure why, but I only measured the 'lists:member` part. BTW, keyfind and keysearch are NIFs too.

bettio

I left some important comments, there are few issues. Still most important: CI must be green.

bettio · 2025-09-04T23:01:29Z

src/libAtomVM/nifs.c

+    term_list_length(list, &proper);
+    if (UNLIKELY(!proper)) {
+        RAISE_ERROR(BADARG_ATOM);
+    }


We can remove this part, so it can be just O(1): instead of using term_list_length to check if it is a proper list or not, we can check it at the end.
So we can also be equivalent to the OTP implementation:

iex(3)> :lists.member(3, [1, 2, 3 | :foo]) true iex(4)> :lists.member(4, [1, 2, 3 | :foo]) ** (ArgumentError) errors were found at the given arguments: * 2nd argument: not a proper list (stdlib 6.2) :lists.member(4, [1, 2, 3 | :foo]) iex:4: (file)

bettio · 2025-09-04T23:03:43Z

src/libAtomVM/nifs.c

+        RAISE_ERROR(BADARG_ATOM);
+    }
+
+    while (!term_is_nil(list)) {


while (term_is_nonempty_list(list))

src/libAtomVM/nifs.c

bettio · 2025-09-04T23:43:23Z

src/libAtomVM/nifs.c

+        RAISE_ERROR(BADARG_ATOM);
+    }
+
+    int n_pos = term_to_int(n);


nitpicking: avm_int_t is the right type for storing term_to_int(n) result.

bettio · 2025-09-04T23:44:59Z

src/libAtomVM/nifs.c

+    }
+
+    int proper;
+    term_list_length(tuple_list, &proper);


same thing as I mentioned earlier.

bettio · 2025-09-04T23:48:30Z

src/libAtomVM/nifs.c

+    return FALSE_ATOM;
+}
+
+static term nif_lists_keysearch(Context *ctx, int argc, term argv[])


This function is deprecated, I'm not sure it is worth it providing a C implementation.

The main reason to add these functions is for compatibility, so I think it should be there 🤔

Caller do not care if a function is provided using a NIF. Whether a function is implemented as a NIF or not is totally transparent to the caller.
For this specific reason it is not an argument if the BEAM has a certain function implemented as a NIF or not.

Reasons for writing a NIF are:

performances

access to native/operating systeam features

So, TL;DR I don't think performance really matters for deprecated functions (except specific situations), so it can be implemented in Erlang, and honestly less C code we have, more I feel happy.

bettio · 2025-09-04T23:55:14Z

src/libAtomVM/defaultatoms.def

 X(MONITORED_BY_ATOM, "\xC", "monitored_by")
+
+X(KEY_ATOM, "\x3", "key")
+X(VALUE_ATOM, "\x5", "value")


Beware the new line at the end of file.

bettio

There are still a few minor quick fixes that are required, 2 of them are required for green CI:

erlfmt (let's format all erl file we changed)
reuse (let's add proper copyright headers)

As soon everything is fixed feel free to squash/fixup everything into a single commit and I will merge it.

tests/erlang_tests/test_lists_keyfind.erl

bettio · 2025-09-17T12:43:29Z

src/libAtomVM/nifs.c

+    return FALSE_ATOM;
+}
+
+static term nif_lists_keysearch(Context *ctx, int argc, term argv[])


Caller do not care if a function is provided using a NIF. Whether a function is implemented as a NIF or not is totally transparent to the caller.
For this specific reason it is not an argument if the BEAM has a certain function implemented as a NIF or not.

Reasons for writing a NIF are:

performances

access to native/operating systeam features

So, TL;DR I don't think performance really matters for deprecated functions (except specific situations), so it can be implemented in Erlang, and honestly less C code we have, more I feel happy.

bettio · 2025-09-17T12:45:49Z

src/libAtomVM/nifs.c

 static term nif_erlang_lists_subtract(Context *ctx, int argc, term argv[]);
 static term nif_zlib_compress_1(Context *ctx, int argc, term argv[]);

+


This empty new line polutes the diff, let's discard it.

bettio

Just a couple of minor changes required. I suggest squashing commits together as soon as you fix them so I can merge this PR very soon :)

bettio · 2025-09-24T13:33:39Z

src/libAtomVM/nifs.c

+    .nif_ptr = nif_erlang_list_to_bitstring_1
+};
+static const struct Nif zlib_compress_nif =
+{


I'm not sure if this is a git rebase glitch or not, anyway let's make sure to rewrite this as:

static const struct Nif zlib_compress_nif = {

Before making any change I suggest squashing everything and rebasing, so clang-format workflow will pass CI.

bettio · 2025-09-24T13:42:22Z

CHANGELOG.md

 - Added `supervisor:which_children/1` and `supervisor:count_children/1`
 - Added `monitored_by` in `process_info/2`
 - Added mock implementation for `current_stacktrace` in `process_info`
+- Added `lists:keyfind`, `lists:keymember`, `lists:member` and `erlang:list_to_bitstring`


This is not accurate: lists:keyfind, lists:keymember, lists:member were already available, so they don't belong to added.
We changed just how they are implemented. So in general, we leave out from the changelog implementation details that are not visible to the user, for those implementation details git log is the right choice.

On the other hand erlang:list_to_bitstring is new, so we can leave it here.

bettio

Let's make clang-format happy and merge this ;) I suggest to squash everything as soon as you fix it, so I can merge it very shortly.

bettio · 2025-09-25T16:41:58Z

src/libAtomVM/nifs.c

    return result;
 }

+


clang-format is not happy about this extra line:

@@ -5844,7 +5844,6 @@ return result; } - static term nif_lists_member(Context *ctx, int argc, term argv[]) { UNUSED(argc)

…nt erlang:list_to_bitstring Signed-off-by: Mateusz Front <mateusz.front@swmansion.com>

bettio reviewed Aug 12, 2025

View reviewed changes

mat-hek changed the title ~~Add lists:keyfind, lists:keymember, lists:keysearch, lists:member and erlang:list_to_bitstring NIFs~~ Add some list-related NIFs Aug 13, 2025

bettio requested changes Sep 4, 2025

View reviewed changes

mat-hek force-pushed the mf/lists-nifs branch 4 times, most recently from 9b384c5 to 5f129fe Compare September 11, 2025 16:48

mat-hek requested a review from bettio September 11, 2025 18:50

bettio requested changes Sep 17, 2025

View reviewed changes

mat-hek force-pushed the mf/lists-nifs branch 3 times, most recently from d6c3c74 to c3b2891 Compare September 24, 2025 10:11

mat-hek requested a review from bettio September 24, 2025 12:49

bettio requested changes Sep 24, 2025

View reviewed changes

mat-hek force-pushed the mf/lists-nifs branch from 6ee6252 to 2f07971 Compare September 25, 2025 15:47

bettio reviewed Sep 25, 2025

View reviewed changes

Rewrite lists:keyfind, lists:keymember, lists:member as NIFs; impleme…

bb901b8

…nt erlang:list_to_bitstring Signed-off-by: Mateusz Front <mateusz.front@swmansion.com>

mat-hek force-pushed the mf/lists-nifs branch from 2f07971 to bb901b8 Compare September 26, 2025 08:47

mat-hek requested a review from bettio September 26, 2025 11:31

bettio merged commit 5409cec into atomvm:main Sep 26, 2025
203 of 208 checks passed

		static term nif_erlang_lists_subtract(Context *ctx, int argc, term argv[]);
		static term nif_zlib_compress_1(Context *ctx, int argc, term argv[]);

Add some list-related NIFs #1795

Add some list-related NIFs #1795

Conversation

mat-hek commented Aug 12, 2025

Uh oh!

bettio left a comment

Choose a reason for hiding this comment

Uh oh!

mat-hek commented Aug 13, 2025

Uh oh!

pguyot commented Aug 17, 2025

Uh oh!

mat-hek commented Aug 18, 2025

Uh oh!

bettio left a comment

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!

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!

bettio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bettio left a comment

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!

bettio left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!