Skip to content

[REACTOR] Cleanup and improve map array containers#462

Merged
tqchen merged 2 commits intoapache:mainfrom
tqchen:map-array-cleanup
Feb 18, 2026
Merged

[REACTOR] Cleanup and improve map array containers#462
tqchen merged 2 commits intoapache:mainfrom
tqchen:map-array-cleanup

Conversation

@tqchen
Copy link
Member

@tqchen tqchen commented Feb 18, 2026

This PR runs the following refactor of the map and array containers.

  • Remove InplaceArrayBase since it is not as useful
  • Extract MapObj into map_base.h so it can be used for other different subclass and variant of map if needed.
  • Add extra helper methods such as clean

We also reorganized SmallBaseMapObj after DenseMapObj as SmallBaseMapObj is a specialization, which can help possible future refactors to make SmallBaseMapObj be aware of DenseMapObj.

The split out of map_base.h is roughly manual copy and a few extra manual edits to reduce unexpected surprises.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @tqchen, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the internal implementation of array and map container classes. The primary goal is to improve modularity and maintainability by separating common map functionalities into a new base class, removing an underutilized array base class, and introducing new utility methods. These changes consolidate shared logic and simplify the container's internal structure without altering external API behavior.

Highlights

  • Refactored Map Container Hierarchy: The core implementation logic for map containers (SmallMapObj and DenseMapObj) has been extracted into a new base class hierarchy (MapBaseObj, SmallMapBaseObj, DenseMapBaseObj) located in map_base.h. The original MapObj now inherits from MapBaseObj, streamlining the design and preparing for future extensions.
  • Removed InplaceArrayBase: The InplaceArrayBase utility class, previously used for array-like memory layouts, has been removed from container_details.h. This simplifies the underlying array implementation.
  • Updated ArrayObj Implementation: The ArrayObj class no longer relies on InplaceArrayBase, and its internal data pointer initialization has been updated to reflect this change, using direct memory offset calculation.
  • Added clear() Helper Method: A new clear() method has been introduced to the map base classes, providing a convenient way to empty map contents and release associated memory.
  • Templated Map Creation and Insertion Methods: Static helper methods for map creation, copying, and insertion (Empty, CreateFromRange, InsertMaybeReHash, CopyFrom) have been updated to accept template arguments for the specific MapObjType, enhancing flexibility and type safety.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • include/tvm/ffi/container/array.h
    • Removed inheritance from details::InplaceArrayBase for ArrayObj.
    • Updated data pointer initialization in make_inplace_array_object to directly calculate memory offset.
    • Removed friend InplaceArrayBase declaration.
  • include/tvm/ffi/container/container_details.h
    • Removed the entire InplaceArrayBase template class definition.
  • include/tvm/ffi/container/map.h
    • Included new header tvm/ffi/container/map_base.h.
    • Changed MapObj to inherit from MapBaseObj instead of Object.
    • Removed extensive map implementation details, delegating them to the new base classes.
    • Updated calls to static map helper methods (Empty, CreateFromRange, InsertMaybeReHash, CopyFrom) to include explicit template arguments.
  • include/tvm/ffi/container/map_base.h
    • Added new file to define MapBaseObj, DenseMapBaseObj, and SmallMapBaseObj.
    • Implemented all core map functionalities (e.g., size, count, at, iterator, Empty, CreateFromRange, InsertMaybeReHash, CopyFrom) within these new base classes.
    • Introduced a clear() method for map objects.
    • Templated static map helper methods to allow specification of the concrete map object type.
    • Adjusted SmallMapBaseObj data initialization to use direct memory offset.
  • include/tvm/ffi/extra/stl.h
    • Updated CopyToMap and MoveToMap methods to use the templated MapObj::CreateFromRange<MapObj>.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@tqchen
Copy link
Member Author

tqchen commented Feb 18, 2026

To simplify the review, the PR is structured in two commits

  • The first commit splits out the related files into map_base.h and cleans up the sequence
    • Copy of MapObj implementation is just copy with no editing
  • The second commit contains the related edits of map_base.h to allow subclass

@tqchen tqchen changed the title [REACTOPR] Cleanup and improve map array containers [REACTOR] Cleanup and improve map array containers Feb 18, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request provides a good cleanup and refactoring of the map and array containers. Removing InplaceArrayBase simplifies the array implementation. Extracting the map implementation into map_base.h with MapBaseObj is a great step towards better modularity and reusability, allowing different map specializations. The addition of the clear() method is also a useful enhancement. The code changes look solid overall. I've found a couple of minor points regarding confusing static_asserts in the new map_base.h that could be improved for better maintainability.

@tqchen tqchen force-pushed the map-array-cleanup branch 2 times, most recently from 1c7f998 to f8d669d Compare February 18, 2026 14:49
@tqchen
Copy link
Member Author

tqchen commented Feb 18, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This PR is a nice refactoring of the map and array containers. Removing InplaceArrayBase and extracting the map implementation into a new map_base.h file are good steps towards better modularity and code clarity. The changes across the modified files are consistent with this goal.

However, I've identified a critical issue in the instantiation logic for DenseMapBaseObj within the new map_base.h file. This issue will likely cause a compilation failure due to an incorrect assumption about object sizes and the use of make_object on a non-registered type. Please see the detailed comment for more information.

@tqchen
Copy link
Member Author

tqchen commented Feb 18, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request is a nice refactoring of the map and array containers. The removal of InplaceArrayBase simplifies the class hierarchy, and extracting the map implementation into map_base.h improves modularity. The changes are consistent and well-executed. I have one minor suggestion to restore a sanity check that was lost during the refactoring.

@tqchen tqchen merged commit 5a6b211 into apache:main Feb 18, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments