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

feature: add support for parallel processor #3280

Merged
merged 11 commits into from
Mar 10, 2020

Conversation

MartinWitt
Copy link
Collaborator

@MartinWitt MartinWitt commented Feb 28, 2020

Fix #2659

The new class allows parallel processing for stateless processors. Subtyping processor allows easy usage in launcher and keeps it simple for the user.
Currently the are still some problems and needed changes:

  • writing doc ( and testing)
  • easier usage => added constructor with Consumer as argument
  • allow usage for non stateless processors => some problems where you need a shared state you could easily share the state between processors by collections. Some problems are never parallel doable. so some work for the user will be left.

wdyt about it?

@monperrus
Copy link
Collaborator

I think it's really cool!

@MartinWitt MartinWitt marked this pull request as ready for review March 3, 2020 18:59
@MartinWitt MartinWitt changed the title WIP: feature: add support for parallel processor feature: add support for parallel processor Mar 4, 2020
@MartinWitt
Copy link
Collaborator Author

i think the feature is finished if no one sees any problems?

@monperrus
Copy link
Collaborator

Great progress.

In Spoon, each time we add a feature, we document it in the doc folder. Could you document the parallel processor in processor.md?

In Spoon, we try to add a line // contract: the tested contract in natural language in each test case. Could you do so?

Thanks!

* @param numberOfProcessors number of concurrent running processors.
* @throws IllegalArgumentException if numberOfProcessors is less than 1.
*/
public AbstractParallelProcessor(Consumer<E> processFunction, int numberOfProcessors) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

neat API!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks

@MartinWitt
Copy link
Collaborator Author

For both failing test cases see #3289 (?) and wait with merges till it is resolved?

@monperrus monperrus merged commit a3126ca into INRIA:master Mar 10, 2020
@monperrus
Copy link
Collaborator

Thanks a lot @MartinWitt

@monperrus
Copy link
Collaborator

For the record, #1111 is related to this.

@monperrus monperrus mentioned this pull request Mar 20, 2020
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.

add support for parallel analysis
2 participants