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 multithreaded test infrastructure with Shuttle #1617

Closed
wants to merge 1 commit into from

Conversation

LucasSte
Copy link

@LucasSte LucasSte commented Jun 5, 2024

Problem

The program cache has multiple synchronization mechanisms without any systemic test infrastructure. Although PR #1471 introduced an adhoc test for program cache, it is insufficient for finding issues in the code.

Summary of Changes

I want to adopt shuttle to run specialized multithreaded test in the CI. In order for it to work, all sync, rand and thread imports must come from shuttle.

@LucasSte LucasSte changed the title Add concurrent test infrastructure with Shuttle Add multithreaded test infrastructure with Shuttle Jun 5, 2024
@alessandrod
Copy link

instead of cfging shuttle everywhere, we could have an internal thread module somewhere that imports the OG stuff or shuttle depending on cfg.

@LucasSte
Copy link
Author

LucasSte commented Jun 5, 2024

instead of cfging shuttle everywhere, we could have an internal thread module somewhere that imports the OG stuff or shuttle depending on cfg.

Do you mean another create in the monorepo? I need the shuttle imports across multiple crates, so a module in one of the existing ones wouldn't work I guess.

@alessandrod
Copy link

instead of cfging shuttle everywhere, we could have an internal thread module somewhere that imports the OG stuff or shuttle depending on cfg.

Do you mean another create in the monorepo? I need the shuttle imports across multiple crates, so a module in one of the existing ones wouldn't work I guess.

Hmm, with some luck there's a crate that is imported by pretty much everything? sdk? :D

@LucasSte
Copy link
Author

LucasSte commented Jun 5, 2024

instead of cfging shuttle everywhere, we could have an internal thread module somewhere that imports the OG stuff or shuttle depending on cfg.

Do you mean another create in the monorepo? I need the shuttle imports across multiple crates, so a module in one of the existing ones wouldn't work I guess.

Hmm, with some luck there's a crate that is imported by pretty much everything? sdk? :D

That would surely work, but enforcing people to use the right import would be difficult.

@alessandrod
Copy link

instead of cfging shuttle everywhere, we could have an internal thread module somewhere that imports the OG stuff or shuttle depending on cfg.

Do you mean another create in the monorepo? I need the shuttle imports across multiple crates, so a module in one of the existing ones wouldn't work I guess.

Hmm, with some luck there's a crate that is imported by pretty much everything? sdk? :D

That would surely work, but enforcing people to use the right import would be difficult.

wouldn't be more difficult than making them import shuttle?

@LucasSte
Copy link
Author

LucasSte commented Jun 5, 2024

instead of cfging shuttle everywhere, we could have an internal thread module somewhere that imports the OG stuff or shuttle depending on cfg.

Do you mean another create in the monorepo? I need the shuttle imports across multiple crates, so a module in one of the existing ones wouldn't work I guess.

Hmm, with some luck there's a crate that is imported by pretty much everything? sdk? :D

That would surely work, but enforcing people to use the right import would be difficult.

wouldn't be more difficult than making them import shuttle?

Fair point. There are drawbacks in both approaches. On one hand, people may forget to add the shuttle import, on the other, they may still import from std::sync, instead of solana_sdk::sync (for example).

@LucasSte
Copy link
Author

LucasSte commented Jun 5, 2024

I'm open to changing then. Anyone else wants to weight in? @Lichtso and @pgarg66 ?

@LucasSte
Copy link
Author

LucasSte commented Jun 6, 2024

Closing this PR in favor of #1634.

@LucasSte LucasSte closed this Jun 6, 2024
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