[
https://issues.apache.org/jira/browse/YARN-6788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16092337#comment-16092337
]
Daniel Templeton commented on YARN-6788:
----------------------------------------
Here's my first-pass comments:
* In {{Resource.equals()}} instead of the nested _for_ loops: {code} for
(ResourceInformation entry : getResources()) {
for (ResourceInformation otherEntry : other.getResources()) {
if (entry.getName().equals(ResourceInformation.MEMORY_MB.getName())
|| entry.getName().equals(ResourceInformation.VCORES.getName())) {
continue;
}
if (entry.getName().equals(otherEntry.getName())) {
if (!entry.equals(otherEntry)) {
return false;
}
}
}
}{code} would it be better to grab the resource index map and iterate
through that instead? If you do that, you can also skip the special casing of
the memory and CPU tests.
* In {{Resource.compareTo()}}, this: {code} ResourceInformation[]
thisResources, otherResources;
thisResources = this.getResources();
otherResources = other.getResources();{code} should be: {code}
ResourceInformation[] thisResources = this.getResources();
ResourceInformation[] otherResources = other.getResources();{code}
* In {{Resource.compareTo()}}, array length is an int, so {{diff}} should be an
int.
* In {{Resource.compareTo()}} we assume that if the number of resource types is
the same, then they're equal. Is that sound? It doesn't seem like that will
produce a consistent sort order. I have the same concern iterating through
rest of the resources. Seems like it should instead be iterating through the
resource type index map.
* {{ResourcePBImpl.getResources()}} should call {{super.getResources()}}
instead of reimplementing the logic.
* {{ResourcePBImpl.getResourceValue()}} should call
{{super.getResourceValue()}} instead of reimplementing the logic.
* {{ResourcePBImpl.getResourceInformation()}} should call
{{super.getResourceInformation()}} instead of reimplementing the logic.
* In {{ResourcePBImpl.mergeLocalToBuilder()}} the _for_ loop should be a _for
each_.
* Seems like you should add a {{ResourceUtils.getResourceType(String
resource)}} method to simplify the code.
* In the {{BaseResource()}} constructor, I don't see a reason to special case
the memory and CPU. Just handle them in the loop with the other resources.
* {{ResourceUtils.indexForResourceInformation}} should be final.
* {{ResourceUtils.getResourceTypesArray()}} should return
{{readOnlyResourcesArray}} instead of recreating it.
* {{ResourceUtils.getResourceTypesMinimumAllocation()}} and
{{ResourceUtils.getResourceTypesMaximumAllocation()}} should use
{{readOnlyResourcesArray}} instead of calling getResourceTypesArray().
* Unrelated to this patch, but
{{ResourceUtils.getResourceTypesMinimumAllocation()}} and
{{ResourceUtils.getResourceTypesMaximumAllocation()}} would be a lot clearer
with an _else_ rather than the _continue_ statements.
* Why not convert {{Resources.FixedValueResource}} to extend {{BaseResource}}?
* In {{TestResourceUtils.testGetResourceInformation()}}, it seems like we
should be able to compare the resource arrays since the order is now fixed
instead of having to compare the maps element by element.
> Improve performance of resource profile branch
> ----------------------------------------------
>
> Key: YARN-6788
> URL: https://issues.apache.org/jira/browse/YARN-6788
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: nodemanager, resourcemanager
> Reporter: Sunil G
> Assignee: Sunil G
> Priority: Blocker
> Attachments: YARN-6788-YARN-3926.001.patch,
> YARN-6788-YARN-3926.002.patch, YARN-6788-YARN-3926.003.patch,
> YARN-6788-YARN-3926.004.patch, YARN-6788-YARN-3926.005.patch,
> YARN-6788-YARN-3926.006.patch
>
>
> Currently we could see a 15% performance delta with this branch.
> Few performance improvements to improve the same.
> Also this patch will handle
> [comments|https://issues.apache.org/jira/browse/YARN-6761?focusedCommentId=16075418&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16075418]
> from [~leftnoteasy].
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
