feature: switch threader_for to int64 indices by Alexandr-Solovev · Pull Request #3471 · uxlfoundation/oneDAL

Alexandr-Solovev · 2026-01-07T13:20:31Z

Description

ABI change: threader functions use 64-bit indices

This change updates the exported threading functions to use int64_t instead of int / int32_t:

_onedal_threader_for
_daal_threader_for

The legacy helper _onedal_threader_for_int64 was removed since the implementation is now unified on 64-bit indices.

Impact

This is an ABI-breaking change:

Function parameter types changed from 32-bit to 64-bit
One exported function was removed

All downstream binaries must be rebuilt.

Release plan

This change is intended for the next major release, where ABI changes are allowed.

Checklist:

Completeness and readability

I have commented my code, particularly in hard-to-understand areas.
I have updated the documentation to reflect the changes or created a separate PR with updates and provided its number in the description, if necessary.
Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
I have resolved any merge conflicts that might occur with the base branch.

Testing

I have run it locally and tested the changes extensively.
All CI jobs are green or I have provided justification why they aren't.
I have extended testing suite if new functionality was introduced in this PR.

Performance

I have measured performance for affected algorithms using scikit-learn_bench and provided at least a summary table with measured data, if performance change is expected.
I have provided justification why performance and/or quality metrics have changed or why changes are not expected.
I have extended the benchmarking suite and provided a corresponding scikit-learn_bench PR if new measurable functionality was introduced in this PR.

david-cortes-intel · 2026-01-07T14:00:52Z

/intelci: run

Vika-F · 2026-01-08T09:29:49Z

The changes look good to me. Please also update the .github/.abignore file, as threading layer is not a part of our ABI, as those functions are not available to users, they are only available internally in oneDAL.

Please add the following section and list the threading symbols there:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;; DAAL THREADING LAYER DESELECTIONS ;;;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
; deselect threading layer functions

[suppress_function]
symbol_name = _daal_threader_for
drop = yes

...

…exandr-Solovev/oneDAL into dev/asolovev_threader_for_upd

Copilot

Pull request overview

This PR implements an ABI-breaking change that migrates threading functions to use 64-bit indices (int64_t) instead of 32-bit (int32_t). The change consolidates the separate threader_for and threader_for_int64 functions into a single unified implementation.

Changes:

Updated _onedal_threader_for and _daal_threader_for function signatures to use int64_t parameters
Removed the deprecated _onedal_threader_for_int64 and _daal_threader_for_int64 functions
Updated all call sites to use the unified threader_for with explicit count parameter

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
cpp/oneapi/dal/detail/threading.hpp	Updated function signatures and removed int64-specific variants
cpp/oneapi/dal/backend/interop/threading.cpp	Updated implementation to use int64_t parameters
cpp/daal/src/threading/threading.h	Changed exported function signatures to int64_t
cpp/daal/src/threading/threading.cpp	Consolidated implementation to single int64_t version
cpp/daal/src/externals/core_threading_win_dll.cpp	Updated Windows DLL function pointer types
cpp/oneapi/dal/table/backend/convert/copy_convert_impl_cpu.cpp	Updated call sites to new API
cpp/oneapi/dal/io/csv/detail/read_graph_kernel_impl.hpp	Updated call sites to new API
cpp/oneapi/dal/algo/basic_statistics/backend/cpu/apply_weights_cpu.cpp	Updated call site to new API
.github/.abignore	Added suppression for removed function in ABI compatibility checks

Copilot · 2026-02-05T12:14:10Z

cpp/oneapi/dal/table/backend/convert/copy_convert_impl_cpu.cpp

    const std::int64_t col_count = shape.second;

-    detail::threader_for_int64(row_count, [&](std::int64_t i) -> void {
+    detail::threader_for(row_count, row_count, [&](std::int64_t i) -> void {


The second parameter appears redundant - passing row_count twice suggests the API might benefit from clearer parameter naming or documentation. Consider whether threads_request should match the iteration count or if this represents a threading hint that should be calculated differently.

Suggested change

detail::threader_for(row_count, row_count, [&](std::int64_t i) -> void {

const std::int64_t threads_request = detail::threader_get_max_threads();

detail::threader_for(row_count, threads_request, [&](std::int64_t i) -> void {

Copilot · 2026-02-05T12:14:10Z

cpp/daal/src/threading/threading.h

-}
-
 /// Pass a function to be executed in a for loop to the threading layer.
 /// The maximal number of iterations in the loop is 2^31 - 1.


The documentation comment is now incorrect - it states the maximum is 2^31 - 1, but the function now accepts int64_t which allows up to 2^63 - 1 iterations. This comment should be updated to reflect the new 64-bit limit.

Bot is right here.

Copilot · 2026-02-05T12:14:11Z

cpp/daal/src/threading/threading.h

@@ -266,34 +265,13 @@ inline void threader_func_break(int i, bool & needBreak, const void * a)
 /// @param[in] reserved Parameter reserved for the future. Currently unused.


The parameter name 'reserved' in the documentation no longer accurately describes the second parameter, which is now 'threads_request' with a specific purpose. The documentation should be updated to reflect that this parameter controls the number of threads requested for parallel execution.

There is a mismatch between the headers and the C++ files regarding the names of the parameters.

Actually, it's a common issue for all threader functions. WIll be update all of the functions

david-cortes-intel

Please address the issues from copilot.

Vika-F · 2026-02-06T13:05:56Z

I do not agree with Copilot that those changes are ABI breaking.
Because libonedal_thread.so contains only internal APIs. Those APIs are not linked directly to user's applications, they are only used internally by libonedal_core.so.

So, changing the APIs/ABI of the threading layer cannot impact the end user.
I propose either to ignore threading APIs in ABI check or exclude threading library entirely from the ABI check.

@Alexandr-Solovev , @david-cortes-intel what do you think?

david-cortes-intel · 2026-02-06T13:09:43Z

I do not agree with Copilot that those changes are ABI breaking. Because libonedal_thread.so contains only internal APIs. Those APIs are not linked directly to user's applications, they are only used internally by libonedal_core.so.

So, changing the APIs/ABI of the threading layer cannot impact the end user. I propose either to ignore threading APIs in ABI check or exclude threading library entirely from the ABI check.

@Alexandr-Solovev , @david-cortes-intel what do you think?

I think copilot took that from the description which states that it's ABI breaking due to removal of an exported function.

Vika-F · 2026-02-06T13:13:58Z

I think copilot took that from the description which states that it's ABI breaking due to removal of an exported function.

@david-cortes-intel But this function is again removed from libonedal_thread.so library, which is not used directly by users.
So, it does break the ABI of the threading lib, but it does not break the ABI of the whole oneDAL as a product.

david-cortes-intel · 2026-02-06T13:15:39Z

I think copilot took that from the description which states that it's ABI breaking due to removal of an exported function.

@david-cortes-intel But this function is again removed from libonedal_thread.so library, which is not used directly by users. So, it does break the ABI of the threading lib, but it does not break the ABI of the whole oneDAL as a product.

Then I guess @Alexandr-Solovev might want to edit the description.

init commit for replace threader_for

a4c06de

Alexandr-Solovev added 4 commits January 29, 2026 16:19

fix abi

4ba268e

Merge branch 'main' into dev/asolovev_threader_for_upd

3a569e7

minor fix

c0f983e

Merge branch 'dev/asolovev_threader_for_upd' of https://github.com/Al…

2a313dc

…exandr-Solovev/oneDAL into dev/asolovev_threader_for_upd

Alexandr-Solovev added the API/ABI breaking change label Feb 5, 2026

Alexandr-Solovev changed the title ~~init commit for replace threader_for~~ feature: switch threader_for to int64 indices Feb 5, 2026

Merge branch 'main' into dev/asolovev_threader_for_upd

6ace7c3

Alexandr-Solovev marked this pull request as ready for review February 5, 2026 12:12

Alexandr-Solovev requested review from ahuber21, avolkov-intel, david-cortes-intel, ethanglaser, homksei, icfaust and napetrov as code owners February 5, 2026 12:12

Copilot AI review requested due to automatic review settings February 5, 2026 12:12

Copilot AI reviewed Feb 5, 2026

View reviewed changes

david-cortes-intel requested changes Feb 5, 2026

View reviewed changes

fixes for int64

aaf4c6a

-    detail::threader_for(row_count, row_count, [&](std::int64_t i) -> void {
+    const std::int64_t threads_request = detail::threader_get_max_threads();
+    detail::threader_for(row_count, threads_request, [&](std::int64_t i) -> void {

		@@ -266,34 +265,13 @@ inline void threader_func_break(int i, bool & needBreak, const void * a)
		/// @param[in] reserved Parameter reserved for the future. Currently unused.

Conversation

Alexandr-Solovev commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

ABI change: threader functions use 64-bit indices

Impact

Release plan

Uh oh!

david-cortes-intel commented Jan 7, 2026

Uh oh!

Vika-F commented Jan 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

david-cortes-intel Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

david-cortes-intel Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Alexandr-Solovev Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

david-cortes-intel left a comment

Choose a reason for hiding this comment

Uh oh!

Vika-F commented Feb 6, 2026

Uh oh!

david-cortes-intel commented Feb 6, 2026

Uh oh!

Vika-F commented Feb 6, 2026

Uh oh!

david-cortes-intel commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Alexandr-Solovev commented Jan 7, 2026 •

edited

Loading