hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel Templeton (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-6788) Improve performance of resource profile branch
Date Tue, 18 Jul 2017 23:40:00 GMT

    [ 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-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message