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

template the erase_if and export_batch_if API #132

Merged
merged 1 commit into from
May 26, 2023

Conversation

Lifann
Copy link
Collaborator

@Lifann Lifann commented May 25, 2023

APIs like erase_if and export_batch_if use function pointer to enable multiple policies to match the keys in table. But It can easily lead to issues like memory access error, misaligned address(#125), performance downgrade.

This PR change the APIs with approach of Templating which avoid issues above.

@github-actions
Copy link

@Lifann Lifann force-pushed the templatorize-api-with-pred branch 5 times, most recently from 5c9d1f3 to a4e478d Compare May 25, 2023 14:29
@Lifann Lifann changed the title Templatorize the erase_if and export_batch_if API template the erase_if and export_batch_if API May 25, 2023
@Lifann Lifann requested a review from rhdong May 25, 2023 15:23
@rhdong
Copy link
Member

rhdong commented May 26, 2023

/blossom-ci

@rhdong
Copy link
Member

rhdong commented May 26, 2023

Hi @Lifann, please drop the commit in the head and rebase on the master. Thank you!

Copy link
Member

@rhdong rhdong left a comment

Choose a reason for hiding this comment

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

LGTM

@rhdong rhdong force-pushed the templatorize-api-with-pred branch from a4e478d to 58d7484 Compare May 26, 2023 02:25
@rhdong
Copy link
Member

rhdong commented May 26, 2023

/blossom-ci

@rhdong rhdong merged commit f7d4fe7 into NVIDIA-Merlin:master May 26, 2023
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