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

[SYSTEMML-2416] Use synchronized method instead of single thread pool #790

Closed
wants to merge 1 commit into from

Conversation

EdgarLGB
Copy link
Member

@EdgarLGB EdgarLGB commented Jun 23, 2018

Hi @mboehm7 ,

Here is the PR for leveraging the synchronized update method instead of the callable method. I think that we do not need the single thread pool because the agg service is always invoked in a synchronized manner.

Thanks for the review,
Guobao

@mboehm7
Copy link
Contributor

mboehm7 commented Jun 23, 2018

LGTM - thanks @EdgarLGB. This is great. There wasn't a significant change in performance (i.e., fluctuations larger than difference) - in these situations, we should always prefer the simpler approach. During the merge, I additionally flattened the aggregation service into the parameter server to reduce unnecessary indirections.

@asfgit asfgit closed this in e7fccd1 Jun 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants