Skip to content
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

Shared Arena Env Allocator Usage Across Modules #20027

Open
RyanRio opened this issue Mar 22, 2024 · 5 comments
Open

Shared Arena Env Allocator Usage Across Modules #20027

RyanRio opened this issue Mar 22, 2024 · 5 comments
Labels
stale issues that have not been addressed in a while; categorized by a bot

Comments

@RyanRio
Copy link

RyanRio commented Mar 22, 2024

Describe the issue

This is a precursor to #19795, I've created a full demo for this one, and will have a full demo for the first issue soon. I'd like to verify that the error code I'm receiving is 1. expected, and 2. safe to ignore.

When an executable (demo.exe) dynamically loads two DLLs (a.dll and b.dll) that separately create a session through usage of a shared static library (lib) the following code causes a ORT_INVALID_ARGUMENT status -

m_ort->CreateArenaCfgV2(keys, reinterpret_cast<const size_t*>(values), sizeof(values) / sizeof(size_t), &arena_cfg);
m_ort->CreateAndRegisterAllocator(GetEnv(), mem_info, arena_cfg); // ERROR HERE in whatever DLL's 'Run' function was called second

Normally to ensure the allocator is only created once in the process, I could for instance use thread local storage indices with TlsAlloc, TlsGetValue, etc, but each module has it's own thread local state. So it seems like the best I can do (without pulling out some of the library to it's own shared DLL which I don't want to do) is ignore the error, which from the logs seems benign as no extra allocations occur.

To reproduce

I've attached a complete example, model and onnxruntime (1.17) -

demo.zip

A few things of note -

  1. I create the env every time (CreateEnvWithGlobalThreadPools), but it seems that as long as I don't call ReleaseEnv until the very end onnx will hand back the first env created
  2. I configure the session to use the env allocator AddSessionConfigEntry(m_session_options, kOrtSessionOptionsConfigUseEnvAllocators, "1")
  3. I disable the cpu mem arena (DisableCpuMemArena(m_session_options)) for the session as without doing so it recreates the BFCArena according to the logs, but then ends up using the one in the env anyway, so this seems undesired
  4. In dll_a I demonstrate that you can use TLS to avoid double initialization within the module

To see the error, uncomment the ORT_ABORT_ON_ERROR macro -

// ORT_ABORT_ON_ERROR(m_ort->CreateAndRegisterAllocator(GetEnv(), mem_info, arena_cfg));
std::cout << "\nCreating and registering allocator..." << std::endl;
m_ort->CreateAndRegisterAllocator(GetEnv(), mem_info, arena_cfg);
std::cout << "Created and registered allocator..." << std::endl;

Urgency

No response

Platform

Windows

OS Version

10

ONNX Runtime Installation

Released Package

ONNX Runtime Version or Commit ID

1.17

ONNX Runtime API

C

Architecture

X64

Execution Provider

Default CPU

Execution Provider Library Version

No response

@github-actions github-actions bot added the platform:windows issues related to the Windows platform label Mar 22, 2024
@snnn
Copy link
Member

snnn commented Mar 22, 2024

CreateEnv should only be called once per process.

@RyanRio
Copy link
Author

RyanRio commented Mar 22, 2024

Is there a way I can query if it's already been created so the two modules can cooperate?

@snnn
Copy link
Member

snnn commented Mar 22, 2024

I'm sorry. No, there isn't such a function.

@RyanRio
Copy link
Author

RyanRio commented Mar 22, 2024

Ah ok, and when you say "should", is it undefined behavior to call it more than once? Same question as for calling CreateAndRegisterAllocatorV2.

I'm also all ears for alternative solutions if anyone has one, although I understand that's not really onnx's problem.

@sophies927 sophies927 removed the platform:windows issues related to the Windows platform label Mar 28, 2024
Copy link
Contributor

github-actions bot commented May 1, 2024

This issue has been automatically marked as stale due to inactivity and will be closed in 30 days if no further activity occurs. If further support is needed, please provide an update and/or more details.

@github-actions github-actions bot added the stale issues that have not been addressed in a while; categorized by a bot label May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale issues that have not been addressed in a while; categorized by a bot
Projects
None yet
Development

No branches or pull requests

3 participants