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

JaggedTensor permute - less CPU ops #2786

Closed
wants to merge 1 commit into from

Conversation

che-sh
Copy link
Contributor

@che-sh che-sh commented Mar 7, 2025

Summary: JaggedTensor.permute could be called with very large indices list (a few hundred items) - so calling python properties self.keys(), self.variable_stride_per_key() and self.stride_per_key_per_rank() in the loop over indices start to compound and take noticeable time on CPU.

Differential Revision: D70609204

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 7, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D70609204

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D70609204

@che-sh che-sh force-pushed the export-D70609204 branch from 0b32aef to 51755cd Compare March 7, 2025 03:00
che-sh added a commit to che-sh/torchrec that referenced this pull request Mar 7, 2025
Summary:

`JaggedTensor.permute` could be called with very large `indices` list (a few hundred items) - so calling python properties `self.keys()`, `self.variable_stride_per_key()` and `self.stride_per_key_per_rank()` in the loop over indices start to compound and take noticeable time **on CPU**.

Differential Revision: D70609204
@che-sh che-sh force-pushed the export-D70609204 branch from 51755cd to c264c54 Compare March 7, 2025 04:33
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D70609204

che-sh added a commit to che-sh/torchrec that referenced this pull request Mar 7, 2025
Summary:
Pull Request resolved: pytorch#2786

`JaggedTensor.permute` could be called with very large `indices` list (a few hundred items) - so calling python properties `self.keys()`, `self.variable_stride_per_key()` and `self.stride_per_key_per_rank()` in the loop over indices start to compound and take noticeable time **on CPU**.

Differential Revision: D70609204
@che-sh che-sh force-pushed the export-D70609204 branch from c264c54 to a3d67d4 Compare March 7, 2025 04:36
@che-sh che-sh force-pushed the export-D70609204 branch from a3d67d4 to cc7219a Compare March 18, 2025 23:11
che-sh added a commit to che-sh/torchrec that referenced this pull request Mar 18, 2025
Summary:

`JaggedTensor.permute` could be called with very large `indices` list (a few hundred items) - so calling python properties `self.keys()`, `self.variable_stride_per_key()` and `self.stride_per_key_per_rank()` in the loop over indices start to compound and take noticeable time **on CPU**.

Reviewed By: sarckk

Differential Revision: D70609204
Summary:
Pull Request resolved: pytorch#2786

`JaggedTensor.permute` could be called with very large `indices` list (a few hundred items) - so calling python properties `self.keys()`, `self.variable_stride_per_key()` and `self.stride_per_key_per_rank()` in the loop over indices start to compound and take noticeable time **on CPU**.

Reviewed By: sarckk

Differential Revision: D70609204
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D70609204

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants