Move NIF memory allocation boundary by iperks · Pull Request #7 · MatchSense/exgboost

iperks · 2026-07-24T08:40:32Z

See details in CHANGELOG.md and CONTRIBUTING.md

Copilot

Pull request overview

This PR refactors the Elixir↔NIF boundary away from JSON-encoded NumPy Array Interface strings and toward passing Array Interface components directly (binary/typestr/shape/readonly), primarily to improve memory safety and remove address-based APIs.

Changes:

Replace JSON Array Interface arguments with component-based arguments across DMatrix creation, info-setting, and prediction NIFs.
Remove pointer/address exposure from EXGBoost.ArrayInterface and adjust tensor reconstruction to use stored binaries.
Add C-side validation helpers (typestr/shape/readonly/binary size) and add/adjust tests + changelog to reflect safety work.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
test/nif_test.exs	Updates NIF tests to use component-based Array Interface calls; adds safety validation tests.
test/exgboost_test.exs	Updates ArrayInterface tests for the new struct shape and tensor reconstruction behavior.
lib/exgboost/nif.ex	Updates NIF specs/docs to accept Array Interface components instead of JSON strings; deprecates unsafe APIs.
lib/exgboost/dmatrix.ex	Updates DMatrix APIs to pass Array Interface components; changes quantile cut handling to consume NIF-returned maps.
lib/exgboost/array_interface.ex	Removes address/tensor caching; reconstructs tensors directly from stored binaries.
lib/exgboost.ex	Updates prediction paths to pass Array Interface components to NIFs for dense/CSR inputs.
CHANGELOG.md	Adds a changelog entry describing safety/interface changes.
c/exgboost/src/utils.c	Adds validation helpers + atom caching and array-interface JSON builder; adjusts integer safety/conversions.
c/exgboost/src/exgboost.c	Updates NIF function arities and initializes cached atoms on load/upgrade.
c/exgboost/src/dmatrix.c	Updates DMatrix NIFs to build Array Interface JSON from components; adds size checks and quantile cut map return.
c/exgboost/src/booster.c	Updates prediction NIFs to build Array Interface JSON from components; adds size checks for VLAs.
c/exgboost/include/utils.h	Declares new helpers and atom init; documents deprecated unsafe APIs.

Comments suppressed due to low confidence (1)

test/nif_test.exs:732

Same endianness issue: float-32-native does not necessarily match typestr "<f4". Use little-endian encoding here so the test is portable.

      small_binary = <<1.0::float-32-native, 2.0::float-32-native>>

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

lib/exgboost/dmatrix.ex:155

slice/3 passes allow_groups as a boolean, but the underlying NIF expects an int (0 or 1). This will cause allow_groups parsing to fail in the NIF when called via EXGBoost.DMatrix.slice/3.

    opts = Keyword.validate!(opts, allow_groups: false)
    allow_groups = Keyword.fetch!(opts, :allow_groups)
    EXGBoost.NIF.dmatrix_slice(dmat.ref, Nx.to_binary(r_index), allow_groups)
  end

  @doc """
  Slice the DMatrix and return a new DMatrix that only contains rindex.
  """
+  @spec slice(any(), Nx.Tensor.t()) :: reference()


  ret =
      exg_ok(env, enif_make_tuple3(
-                      env, enif_make_list_from_array(env, indptr, num_rows + 1),
-                      enif_make_list_from_array(env, indices, num_non_missing),
-                      enif_make_list_from_array(env, data, num_non_missing)));
+                      env, enif_make_list_from_array(env, indptr, (size_t)(num_rows + 1)),
+                      enif_make_list_from_array(env, indices, (size_t)num_non_missing),
+                      enif_make_list_from_array(env, data, (size_t)num_non_missing)));


+    while (pos + needed + 2 >= capacity) { // +2 for ']' and '\0'
+      capacity *= 2;
+      char *new_json = enif_realloc(json, capacity);
+      if (new_json == NULL) {
+        enif_free(json);
+        *error_msg = "Failed to reallocate memory for shape JSON";
+        return 0;
+      }
+      json = new_json;
+    }


  @type dmatrix_reference :: reference()
  @type booster_reference :: reference()
  @type exgboost_return_type(return_type) :: {:ok, return_type} | {:error, String.t()}
+  @type feature_score_result :: {[String.t()], tuple(), [float()]}


-        %{data: [4418559984, true], shape: [2, 3], typestr: "<i8", version: 3}
+  ## Examples
+
+      iex(1)> tensor = EXGBoost.ArrayInterface.from_tensor(Nx.tensor([[1,2,3],[4,5,6]]))


iperks added 7 commits July 24, 2026 08:43

Mark get_binary_address and get_binary_from_address as deprecated

f444a7a

Prefer platform-independent integer conversions

0868356

Initialize true/false atoms in NIF load

fdfd592

Add some memory safety checks to prevent OOM issues

56382f6

Move address derivation and JSON construction into NIF layer

70dfddd

Institute a CHANGELOG

b0bc961

Formatting pass

7d32524

iperks requested a review from Copilot July 24, 2026 08:51

Copilot started reviewing on behalf of iperks July 24, 2026 08:52 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

iperks and others added 10 commits July 24, 2026 10:02

Update comment to match new behaviour

6caf22a

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Change wording in CHANGELOG after moving tests

d74c5f7

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Add missing C header enabling atoi usage

872e4a9

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Use enif_make_new_binary for cleaner memory management

98c8cb7

Add CONTRIBUTING markdown

b026670

Remove padding

73cefff

Simplify endianness check

fa0b350

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Better typestr conversion

9613a30

More edge case tests

1d7616e

Order keys when inspected

d22a9b9

iperks changed the title ~~Ip/dmatrix interface~~ Move NIF memory allocation boundary Jul 24, 2026

iperks marked this pull request as ready for review July 24, 2026 10:32

iperks added 8 commits July 24, 2026 11:53

Make :ok and :error atoms static

2525914

Change doctest test shape

d1e8d30

Further hardening and improvements around typestr parsing

bb4e547

CHANGELOG updates

a27a967

Pass array interfaces in as tuples to reduce arity complexity

93d9214

Add prediction stability test

2a5e5ea

Fixed little-endian typestr

3ea49ab

Prefer yyjson for JSON data extraction

da4a790

iperks mentioned this pull request Jul 24, 2026

Fix non-deterministic predictions caused by GC-invalidated binary addresses #6

Closed

iperks added 3 commits July 24, 2026 15:44

Remove unused EXGDMatrixSetDenseInfo

5181aa4

Replace variable-length stack array creation

bb11ac2

Backfill test, fix EXGDMatrixGetStrFeatureInfo

0ed6b5d

iperks requested a review from Copilot July 24, 2026 15:11

Copilot started reviewing on behalf of iperks July 24, 2026 15:11 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

iperks added 2 commits July 29, 2026 16:10

Remove weak test, we've already implemented a better one

21f7ed3

Delete unsafe get_binary_address and get_binary_from_address from NIF

54be361

iperks force-pushed the ip/dmatrix-interface branch from 35b7fbb to 54be361 Compare July 29, 2026 15:35

Bump to 0.10.0 - BREAKING

f480a1c

iperks merged commit bef912b into main Jul 30, 2026
7 checks passed

iperks deleted the ip/dmatrix-interface branch July 30, 2026 14:49

Navigation Menu

Uh oh!

Move NIF memory allocation boundary - #7

Move NIF memory allocation boundary#7
iperks merged 31 commits into
mainfrom
ip/dmatrix-interface

iperks commented Jul 24, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

iperks commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

iperks commented Jul 24, 2026 •

edited

Loading