feat!: [codex] support polymorphic handles and configs - #917
Draft
voltjia wants to merge 1 commit into
Draft
Conversation
voltjia
force-pushed
the
codex/polymorphic-handle-config
branch
from
August 11, 2026 03:37
f9a26ee to
8142a51
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cloneable<Base, Derived>CRTP helper for concreteConfigandHandlesubclasses.ConfigandHandlevirtual destructors and virtualClone()methods while keeping both base classes concrete.OperatorBasevalue members with ownedstd::unique_ptrclones; existingset_config(const Config&)andset_handle(const Handle&)signatures stay unchanged.Motivation
Operator-specific options currently have to be added to the shared
src/config.h, because assigning a derivedConfigorHandletoOperatorBaseslices its dynamic state. This PR provides only the ownership mechanism needed for operator-specific subclasses.This change is deliberately independent of #800 and contains no Triton/JIT, cache-key, constructor-dispatch, or handle-borrowing optimization changes.
Type of Change
feat- polymorphicConfigandHandleownershipfix- bug fixperf- performance improvementrefactor- code restructuring without behavior changetest- focused C++ API coveragedocs- documentation onlybuild/ci- build system or CI configurationchore- tooling, formatting, or other non-code changesPlatforms Affected
The shared public types and
OperatorBaseare used by every backend.WITH_CPU)WITH_NVIDIA)WITH_ILUVATAR)WITH_METAX)WITH_CAMBRICON)WITH_MOORE)WITH_ASCEND)WITH_TORCH)Smoke Test Result
Validated from commit
8142a511cf9cdac79d31f2247750a3344c2f7c20:GitHub Actions provides the installed-library and backend build/run coverage. All required checks passed, including clang-format, Ruff, documentation build, and unit plus shadow jobs for NVIDIA, Iluvatar, MetaX, Moore, Cambricon, and Ascend.
Test Results on Supported Platforms
Benchmark / Performance Impact
No performance optimization is included.
Configis cloned when an operator is constructed, which normally happens on a cache miss. The existing invocation path callsset_handle()for every invocation; after this change that performs one virtualClone(), one derived-object copy, and one heap allocation per call. A borrowed-handle fast path can be evaluated separately without coupling it to the polymorphism API.Notes for Reviewers
ConfigandHandlegain virtual tables, and the protectedOperatorBase::config_/handle_members change from values tostd::unique_ptr.final;Cloneableenforces this whenClone()is instantiated.ConfigandHandleclasses remain directly constructible and clone themselves by default.Configfields affect cached behavior must include those fields explicitly in its existingCacheKeyBuilder<Key>specialization.set_config()andset_handle()remain replaceable, matching the previous setter semantics; no one-time initialization rule is added.