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

Change IFRT and PjRt layout API to return std::shared_ptr<const xla::PjRtLayout> instead of std::unique_ptr<xla::PjRtLayout> #21012

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

copybara-service[bot]
Copy link

@copybara-service copybara-service bot commented Jan 4, 2025

Change IFRT and PjRt layout API to return std::shared_ptr<const xla::PjRtLayout> instead of std::unique_ptr<xla::PjRtLayout>

The current API design that uses std::unique_ptr<xla::PjRtLayout> has several issues:

  • The API requires xla::PjRtLayout to be copied in some scenarios, e.g., xla::ifrt::Array internally stores a layout and returns its copy every time layout() is called. This forces implementations to break the abstraction boundary because xla::PjRtLayout is an abstract class and std::unique_ptr<xla::PjRtLayout> is not copyable. The current implementation either stores xla::Layout and creates xla::PjRtLayout every time, or downcasts xla::PjRtLayout to xla::PjRtXlaLayout to perform the copy.
  • xla::Layout is expensive to copy (sizeof(xla::Layout) is 248 bytes as of 2025-01-03) and copying xla::PjRtXlaLayout requires copying or moving xla::Layout.

To address these two problems, this CL changes PjRt and IFRT APIs that return xla::PjRtLayout to instead use std::shared_ptr<const xla::PjRtLayout>, so that PjRt layouts can be cheaply copied. Similar patterns have been used in other places such as xla::ifrt::Sharding and xla::PjRtExecutable::GetHloModules().

Some implementations have been updated to take advantage of this change. For example, PjRtCApiBuffer::layout() no longer performs a layout copy and instead reuses an internally cached instance of std::shared_ptr<const xla::PjRtLayout>.

@copybara-service copybara-service bot force-pushed the test_711892970 branch 2 times, most recently from c358bd9 to 0e1e739 Compare January 4, 2025 01:55
@copybara-service copybara-service bot changed the title Return std::shared_ptr<const xla::PjRtLayout> from IFRT and PjRt instead of std::unique_ptr<xla::PjRtLayout> Change IFRT and PjRt layout API to return std::shared_ptr<const xla::PjRtLayout> instead of std::unique_ptr<xla::PjRtLayout> Jan 4, 2025
@copybara-service copybara-service bot force-pushed the test_711892970 branch 12 times, most recently from c23a050 to 91b6581 Compare January 6, 2025 20:31
…:PjRtLayout>` instead of `std::unique_ptr<xla::PjRtLayout>`

The current API design that uses `std::unique_ptr<xla::PjRtLayout>` has several issues:

* The API requires `xla::PjRtLayout` to be copied in some scenarios, e.g., `xla::ifrt::Array` internally stores a layout and returns its copy every time `layout()` is called. This forces implementations to break the abstraction boundary because `xla::PjRtLayout` is an abstract class and `std::unique_ptr<xla::PjRtLayout>` is not copyable. The current implementation either stores `xla::Layout` and creates `xla::PjRtLayout` every time, or downcasts `xla::PjRtLayout` to `xla::PjRtXlaLayout` to perform the copy.
* `xla::Layout` is expensive to copy (`sizeof(xla::Layout)` is 248 bytes as of 2025-01-03) and copying `xla::PjRtXlaLayout` requires copying or moving `xla::Layout`.

To address these two problems, this CL changes PjRt and IFRT APIs that return `xla::PjRtLayout` to instead use `std::shared_ptr<const xla::PjRtLayout>`, so that PjRt layouts can be cheaply copied. Similar patterns have been used in other places such as `xla::ifrt::Sharding` and `xla::PjRtExecutable::GetHloModules()`.

Some implementations have been updated to take advantage of this change. For example, `PjRtCApiBuffer::layout()` no longer performs a layout copy and instead reuses an internally cached instance of `std::shared_ptr<const xla::PjRtLayout>`.

PiperOrigin-RevId: 712645581
@copybara-service copybara-service bot merged commit fe28dbd into main Jan 6, 2025
@copybara-service copybara-service bot deleted the test_711892970 branch January 6, 2025 21:52
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.

1 participant