You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by "elihschiff (via GitHub)" <gi...@apache.org> on 2023/05/03 17:19:02 UTC
[GitHub] [yunikorn-core] elihschiff opened a new pull request, #536: [YUNIKORN-1720] Improve the performance of the node.preAllocateCheck() function
elihschiff opened a new pull request, #536:
URL: https://github.com/apache/yunikorn-core/pull/536
### What is this PR for?
The node.preAllocateCheck() function gets run a lot during the scheduling cycle. The function is effectively a boolean function, either the node can be allocated on or it cannot. Instead of returning a bool it returns nil for true and a full error message for false. The thing is, that error message is never read anywhere. From what I can tell, the only thing the code does is check if there is an error message but not the contents.
I ran a profiler on the code and noticed that about 25% of the scheduling cycle was spend on generating these error messages that are not used. By switching to a bool for the return value it should improve the performance of this one function and therefore the entire scheduling cycle overall.
### What type of PR is it?
* [ ] - Bug Fix
* [x] - Improvement
* [ ] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
N/A
### What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-1720
### How should this be tested?
I have it running on my end and it seems to work. The unit tests also pass after I updated them to look at the new boolean value over the old error message.
### Screenshots (if appropriate)
Before this change (note the large `(*Resource).String (resources.go:L#110)` section under the highlighted function):
<img width="1134" alt="image" src="https://user-images.githubusercontent.com/18558130/235989846-ae798dc3-5e3f-4d52-84e7-64d171b5e940.png">
After this change:
<img width="1128" alt="image" src="https://user-images.githubusercontent.com/18558130/235989957-a8159b5e-ed50-4008-8112-8644544da5b8.png">
### Questions:
* [ ] - The licenses files need update.
* [ ] - There is breaking changes for older versions.
* [ ] - It needs documentation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] codecov[bot] commented on pull request #536: [YUNIKORN-1720] Improve the performance of the node.preAllocateCheck() function
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #536:
URL: https://github.com/apache/yunikorn-core/pull/536#issuecomment-1533489270
## [Codecov](https://app.codecov.io/gh/apache/yunikorn-core/pull/536?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#536](https://app.codecov.io/gh/apache/yunikorn-core/pull/536?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cfd74a8) into [master](https://app.codecov.io/gh/apache/yunikorn-core/commit/1a86bb32ccb06c61248eb5f5011952209a5783f2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1a86bb3) will **increase** coverage by `0.01%`.
> The diff coverage is `85.71%`.
```diff
@@ Coverage Diff @@
## master #536 +/- ##
==========================================
+ Coverage 75.03% 75.05% +0.01%
==========================================
Files 70 70
Lines 11334 11330 -4
==========================================
- Hits 8505 8504 -1
+ Misses 2531 2529 -2
+ Partials 298 297 -1
```
| [Impacted Files](https://app.codecov.io/gh/apache/yunikorn-core/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/scheduler/objects/application.go](https://app.codecov.io/gh/apache/yunikorn-core/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uLmdv) | `64.42% <50.00%> (ø)` | |
| [pkg/scheduler/objects/node.go](https://app.codecov.io/gh/apache/yunikorn-core/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL25vZGUuZ28=) | `81.92% <100.00%> (-0.21%)` | :arrow_down: |
... and [1 file with indirect coverage changes](https://app.codecov.io/gh/apache/yunikorn-core/pull/536/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko closed pull request #536: [YUNIKORN-1720] Improve the performance of the node.preAllocateCheck() function
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko closed pull request #536: [YUNIKORN-1720] Improve the performance of the node.preAllocateCheck() function
URL: https://github.com/apache/yunikorn-core/pull/536
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org