Skip to content

Conversation

@rupengliu-meta
Copy link
Contributor

@rupengliu-meta rupengliu-meta commented Dec 4, 2025

Description

Discussed with @bythew3i offline! This commit removes one control flow branch in path of prefetching bkv. Around 3% kernel speedup before tuning! (since I don't have tuning script, so unable to report updated speedup)

If the change fixes a bug or a Github issue, please include a link, e.g.,:
FIXES: b/123456
FIXES: #123456

Tests

E2E tests
截屏2025-12-03 下午4 11 34
Integration tests:
截屏2025-12-03 下午4 41 13
截屏2025-12-03 下午4 56 33

Please describe how you tested this change, and include any instructions and/or
commands to reproduce.

Checklist

Before submitting this PR, please make sure:

  • I have performed a self-review of my code.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have made or will make corresponding changes to any relevant documentation.

Signed-off-by: rupengliu-meta <rupengliu@meta.com>
sem,
wait,
)
size = lax.select(bkv_sz_frm_new > 0, bkv_sz_frm_new, 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

so if size==0, then the dma will be a no-op?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, exactly

Copy link
Collaborator

@kyuyeunk kyuyeunk left a comment

Choose a reason for hiding this comment

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

Lgtm but requires approval from @bythew3i

@kyuyeunk kyuyeunk added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 6, 2025
@kyuyeunk
Copy link
Collaborator

kyuyeunk commented Dec 6, 2025

Also, can you do a rebase instead of merge to update the branch? There's a weird bug where pre merge ci doesn't run if you do merge.

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

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants