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

Recompute linear_cache_indices for pipeline prefetching #2147

Closed
wants to merge 1 commit into from

Conversation

sryap
Copy link
Contributor

@sryap sryap commented Nov 21, 2023

Summary:
When pipeline prefetching is enabled (prefetch_pipeline=True) for
EmbeddingLocation.MANAGED_CACHING, TBE has to update
lxu_cache_locations to ensure cache consistency before the backward
pass. The lxu_cache_locations update requires
linear_cache_indices as an input. Prior to this diff, TBE keeps
linear_cache_indices alive after prefetching until the tensor is
used for the lxu_cache_locations update. This puts a lot of
pressure to the memory space requirement limiting the enablement of
pipeline prefetching for some models. This diff addresses the memory
limitation issue by recomputing linear_cache_indices when it is
needed.

Differential Revision: D50983176

@facebook-github-bot
Copy link
Contributor

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

Copy link

netlify bot commented Nov 21, 2023

Deploy Preview for pytorch-fbgemm-docs canceled.

Name Link
🔨 Latest commit 6c8e0a6
🔍 Latest deploy log https://app.netlify.com/sites/pytorch-fbgemm-docs/deploys/655c2d1264ff02000898e20a

sryap pushed a commit to sryap/FBGEMM that referenced this pull request Nov 21, 2023
Summary:

When pipeline prefetching is enabled (`prefetch_pipeline=True`) for
`EmbeddingLocation.MANAGED_CACHING`, TBE has to update
`lxu_cache_locations` to ensure cache consistency before the backward
pass.  The `lxu_cache_locations` update requires
`linear_cache_indices` as an input.  Prior to this diff, TBE keeps
`linear_cache_indices` alive after prefetching until the tensor is
used for the `lxu_cache_locations` update.  This puts a lot of
pressure to the memory space requirement limiting the enablement of
pipeline prefetching for some models.  This diff addresses the memory
limitation issue by recomputing `linear_cache_indices` when it is
needed.

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

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

Summary:

When pipeline prefetching is enabled (`prefetch_pipeline=True`) for
`EmbeddingLocation.MANAGED_CACHING`, TBE has to update
`lxu_cache_locations` to ensure cache consistency before the backward
pass.  The `lxu_cache_locations` update requires
`linear_cache_indices` as an input.  Prior to this diff, TBE keeps
`linear_cache_indices` alive after prefetching until the tensor is
used for the `lxu_cache_locations` update.  This puts a lot of
pressure to the memory space requirement limiting the enablement of
pipeline prefetching for some models.  This diff addresses the memory
limitation issue by recomputing `linear_cache_indices` when it is
needed.

Reviewed By: jspark1105

Differential Revision: D50983176
sryap added a commit to sryap/FBGEMM that referenced this pull request Nov 21, 2023
Summary:

When pipeline prefetching is enabled (`prefetch_pipeline=True`) for
`EmbeddingLocation.MANAGED_CACHING`, TBE has to update
`lxu_cache_locations` to ensure cache consistency before the backward
pass.  The `lxu_cache_locations` update requires
`linear_cache_indices` as an input.  Prior to this diff, TBE keeps
`linear_cache_indices` alive after prefetching until the tensor is
used for the `lxu_cache_locations` update.  This puts a lot of
pressure to the memory space requirement limiting the enablement of
pipeline prefetching for some models.  This diff addresses the memory
limitation issue by recomputing `linear_cache_indices` when it is
needed.

Reviewed By: jspark1105

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

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

sryap added a commit to sryap/FBGEMM that referenced this pull request Nov 21, 2023
Summary:

When pipeline prefetching is enabled (`prefetch_pipeline=True`) for
`EmbeddingLocation.MANAGED_CACHING`, TBE has to update
`lxu_cache_locations` to ensure cache consistency before the backward
pass.  The `lxu_cache_locations` update requires
`linear_cache_indices` as an input.  Prior to this diff, TBE keeps
`linear_cache_indices` alive after prefetching until the tensor is
used for the `lxu_cache_locations` update.  This puts a lot of
pressure to the memory space requirement limiting the enablement of
pipeline prefetching for some models.  This diff addresses the memory
limitation issue by recomputing `linear_cache_indices` when it is
needed.

Reviewed By: jspark1105

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

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

sryap added a commit to sryap/FBGEMM that referenced this pull request Nov 21, 2023
Summary:

When pipeline prefetching is enabled (`prefetch_pipeline=True`) for
`EmbeddingLocation.MANAGED_CACHING`, TBE has to update
`lxu_cache_locations` to ensure cache consistency before the backward
pass.  The `lxu_cache_locations` update requires
`linear_cache_indices` as an input.  Prior to this diff, TBE keeps
`linear_cache_indices` alive after prefetching until the tensor is
used for the `lxu_cache_locations` update.  This puts a lot of
pressure to the memory space requirement limiting the enablement of
pipeline prefetching for some models.  This diff addresses the memory
limitation issue by recomputing `linear_cache_indices` when it is
needed.

Reviewed By: jspark1105

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

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 37111f5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants