From 21d9bdd8cf54b6113ec2a148cacdab72367fd264 Mon Sep 17 00:00:00 2001 From: ErlendA Date: Mon, 2 Mar 2026 12:42:09 +0100 Subject: [PATCH 1/3] Remove lock when Disabled Workspace mode is set. --- backends/xnnpack/runtime/XNNWorkspace.h | 8 +++++++ .../xnnpack/runtime/XNNWorkspaceManager.cpp | 1 + .../test/runtime/test_workspace_manager.cpp | 24 +++++++++++++++++++ 3 files changed, 33 insertions(+) diff --git a/backends/xnnpack/runtime/XNNWorkspace.h b/backends/xnnpack/runtime/XNNWorkspace.h index 507953a10ab..b7ef442c460 100644 --- a/backends/xnnpack/runtime/XNNWorkspace.h +++ b/backends/xnnpack/runtime/XNNWorkspace.h @@ -34,6 +34,9 @@ class XNNWorkspace { XNNWorkspace& operator=(XNNWorkspace&&) = delete; std::pair, xnn_workspace_t> acquire() { + if (!lock_required_) { + return {std::unique_lock{}, workspace_.get()}; + } auto lock = std::unique_lock(mutex_); return {std::move(lock), workspace_.get()}; } @@ -52,6 +55,10 @@ class XNNWorkspace { return id_; } + void disable_locking() { + lock_required_ = false; + } + static runtime::Result> create() { // Because this class can't be moved, we need to construct it in-place. xnn_workspace_t workspace = nullptr; @@ -72,6 +79,7 @@ class XNNWorkspace { static inline std::atomic next_id_{0}; std::mutex mutex_; uint64_t id_; + bool lock_required_ = true; WorkspacePtr workspace_; }; diff --git a/backends/xnnpack/runtime/XNNWorkspaceManager.cpp b/backends/xnnpack/runtime/XNNWorkspaceManager.cpp index d8c6dae4d6d..5af3395ed89 100644 --- a/backends/xnnpack/runtime/XNNWorkspaceManager.cpp +++ b/backends/xnnpack/runtime/XNNWorkspaceManager.cpp @@ -56,6 +56,7 @@ XNNWorkspaceManager::get_or_create_workspace(uintptr_t program_id) const { return create_result.error(); } + create_result.get()->disable_locking(); return create_result.get(); } else if (mode == WorkspaceSharingMode::PerModel) { return get_or_create_model_workspace(program_id); diff --git a/backends/xnnpack/test/runtime/test_workspace_manager.cpp b/backends/xnnpack/test/runtime/test_workspace_manager.cpp index 8d3203f3f40..a7689966635 100644 --- a/backends/xnnpack/test/runtime/test_workspace_manager.cpp +++ b/backends/xnnpack/test/runtime/test_workspace_manager.cpp @@ -107,6 +107,18 @@ TEST_F(XNNWorkspaceManagerTest, DisabledMode) { workspace2->unsafe_get_workspace(), workspace3->unsafe_get_workspace()); } +TEST_F(XNNWorkspaceManagerTest, DisabledModeAcquireDoesNotLock) { + workspace_manager_->set_sharing_mode(WorkspaceSharingMode::Disabled); + + auto workspace_result = workspace_manager_->get_or_create_workspace(12345); + ASSERT_TRUE(workspace_result.ok()); + auto workspace = workspace_result.get(); + + auto [lock, ptr] = workspace->acquire(); + ASSERT_NE(ptr, nullptr); + EXPECT_FALSE(lock.owns_lock()); +} + TEST_F(XNNWorkspaceManagerTest, PerModelMode) { // In PerModel mode, calls with the same program_id should return the same // workspace. @@ -139,6 +151,18 @@ TEST_F(XNNWorkspaceManagerTest, PerModelMode) { workspace1->unsafe_get_workspace(), workspace3->unsafe_get_workspace()); } +TEST_F(XNNWorkspaceManagerTest, PerModelAcquireStillLocks) { + workspace_manager_->set_sharing_mode(WorkspaceSharingMode::PerModel); + + auto workspace_result = workspace_manager_->get_or_create_workspace(12345); + ASSERT_TRUE(workspace_result.ok()); + auto workspace = workspace_result.get(); + + auto [lock, ptr] = workspace->acquire(); + ASSERT_NE(ptr, nullptr); + EXPECT_TRUE(lock.owns_lock()); +} + TEST_F(XNNWorkspaceManagerTest, GlobalMode) { // In Global mode, all calls should return the same workspace. workspace_manager_->set_sharing_mode(WorkspaceSharingMode::Global); From 7f4bb96387784b1af11a75a20e62d8914a88f892 Mon Sep 17 00:00:00 2001 From: ErlendA Date: Mon, 2 Mar 2026 13:12:00 +0100 Subject: [PATCH 2/3] Run xnnpack workspace disable/enable lock test --- backends/xnnpack/test/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/backends/xnnpack/test/CMakeLists.txt b/backends/xnnpack/test/CMakeLists.txt index 395fb01d189..68fca2ee578 100644 --- a/backends/xnnpack/test/CMakeLists.txt +++ b/backends/xnnpack/test/CMakeLists.txt @@ -18,6 +18,7 @@ set(EXECUTORCH_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/../../..) include(${EXECUTORCH_ROOT}/tools/cmake/Test.cmake) set(_test_srcs runtime/test_xnnexecutor.cpp + runtime/test_workspace_manager.cpp ${EXECUTORCH_ROOT}/extension/threadpool/test/threadpool_test.cpp ) From 019327bfd9f8890b232afcd47ec0b1ce283a67fc Mon Sep 17 00:00:00 2001 From: ErlendA Date: Wed, 4 Mar 2026 09:41:15 +0100 Subject: [PATCH 3/3] Fix lint error --- backends/xnnpack/test/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backends/xnnpack/test/CMakeLists.txt b/backends/xnnpack/test/CMakeLists.txt index 68fca2ee578..3d9c77d6ad6 100644 --- a/backends/xnnpack/test/CMakeLists.txt +++ b/backends/xnnpack/test/CMakeLists.txt @@ -17,8 +17,7 @@ set(EXECUTORCH_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/../../..) include(${EXECUTORCH_ROOT}/tools/cmake/Test.cmake) -set(_test_srcs runtime/test_xnnexecutor.cpp - runtime/test_workspace_manager.cpp +set(_test_srcs runtime/test_xnnexecutor.cpp runtime/test_workspace_manager.cpp ${EXECUTORCH_ROOT}/extension/threadpool/test/threadpool_test.cpp )