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

CPU optimizations - 'hist' method #5104

Closed
SmirnovEgorRu opened this issue Dec 9, 2019 · 3 comments
Closed

CPU optimizations - 'hist' method #5104

SmirnovEgorRu opened this issue Dec 9, 2019 · 3 comments

Comments

@SmirnovEgorRu
Copy link
Contributor

SmirnovEgorRu commented Dec 9, 2019

Moved discussion from PR #5008 to the issue. After the PR - performance drop on CPU is observed. I prepared a plan how to establish optimizations from PR #4529 in master branch again.

I tried to split large changes to separate PRs:

Description Status
1 Extend Performance Timer to track overall performance of each algorithmic stage for whole training Merged
2 Add benchmarks to track performance changes (~3 data sets are enough for the start, it can be extended later) Merged
3 Add nested parallelism in EvaluateSplit function + tests Merged
4 Add nested parallelism in BuildHist function + tests Merged
5 Add nested parallelism in ApplySplit function + tests Merged

Expected timelines for creation the all PRs - EOY.

@trivialfis
Copy link
Member

I would love to see the performance optimization back and sorry for the oversight. Feel free to ping me if you need any help.

@hcho3
Copy link
Collaborator

hcho3 commented Dec 9, 2019

Add benchmarks to track performance changes (~3 data sets are enough for the start, it can be extended later)

For now, a script that we can run is okay, so that performance results can be re-produced. I plan to design and build an automatic system for performance benchmarking.

@SmirnovEgorRu
Copy link
Contributor Author

All PRs are merged, final results:

Dataset higgs1m Letters Airline-ohe MNIST MSRank-30K Mortgage
Before commit #5008, sec 15.5 10.3 55.3 69.5 99.1 18.3
After commit #5008, sec 123.5 144.6 133.2 Inf 332.2 53.4
Current Master, sec 14.7 10.1 59.4 80.5 111.6 19.0

Current master is is slightly slower on MNIST data set than it was before, but it can be covered in the next PRs

Thank you all who helped me to refactor the optimizations to have them in the master again (@hcho3, @trivialfis, @RAMitchell ).

@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants