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

add sanitizer #17

Merged
merged 10 commits into from Oct 14, 2020
Merged

add sanitizer #17

merged 10 commits into from Oct 14, 2020

Conversation

skyzh
Copy link
Member

@skyzh skyzh commented Oct 13, 2020

Signed-off-by: Alex Chi iskyzh@gmail.com

fix #10

Sanitizer will only run when there's any change in skiplist module. First successful run is triggered along with split sanitizer commit.

In the future, we could apply sanitizer to other modules.

Signed-off-by: Alex Chi <iskyzh@gmail.com>
Signed-off-by: Alex Chi <iskyzh@gmail.com>
@skyzh skyzh changed the title add sanitizer [WIP] add sanitizer Oct 13, 2020
Signed-off-by: Alex Chi <iskyzh@gmail.com>
Signed-off-by: Alex Chi <iskyzh@gmail.com>
Signed-off-by: Alex Chi <iskyzh@gmail.com>
Signed-off-by: Alex Chi <iskyzh@gmail.com>
@skyzh skyzh marked this pull request as ready for review October 13, 2020 09:06
@skyzh skyzh changed the title [WIP] add sanitizer add sanitizer Oct 13, 2020
@skyzh skyzh mentioned this pull request Oct 13, 2020
31 tasks
@skyzh skyzh requested a review from BusyJay October 13, 2020 09:13
@skyzh skyzh added this to In progress in agatedb project plan Oct 13, 2020
Copy link
Member

@BusyJay BusyJay left a comment

Choose a reason for hiding this comment

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

Why limit to skiplist only?


proto: proto/proto/meta.proto proto/proto/rustproto.proto
Copy link
Member

Choose a reason for hiding this comment

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

Why remove this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now we use prost-build to generate meta.rs on cargo build, instead of generating them manually.

@skyzh
Copy link
Member Author

skyzh commented Oct 14, 2020

Why limit to skiplist only?

Run all sanitizer bench takes ~40mins, and unsafe code is mostly used in skiplists. So I choose to run sanitizer bench only on there's any change in skiplist.

Maybe we could run this bench manually instead of triggering by PR or commit, and I could include all modules in bench?

@BusyJay
Copy link
Member

BusyJay commented Oct 14, 2020

Maybe we could run this bench manually

Seems more practical.

Signed-off-by: Alex Chi <iskyzh@gmail.com>
@skyzh
Copy link
Member Author

skyzh commented Oct 14, 2020

I've updated the CI config. After this PR being merged, we could manually trigger sanitizer test and bench on GitHub Action webpage.

@skyzh skyzh requested a review from BusyJay October 14, 2020 06:37
Signed-off-by: Alex Chi <iskyzh@gmail.com>
Signed-off-by: Alex Chi <iskyzh@gmail.com>
@skyzh skyzh merged commit 38e99a8 into master Oct 14, 2020
agatedb project plan automation moved this from In progress to Done Oct 14, 2020
@skyzh skyzh deleted the add-sanitizer branch October 14, 2020 08:27
@skyzh
Copy link
Member Author

skyzh commented Oct 14, 2020

image

Now benchmark can be triggered in Action tab

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

Successfully merging this pull request may close these issues.

Add sanitizer check
2 participants