cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daan Hoogland <daan.hoogl...@gmail.com>
Subject Re: Review Request 23194: Fixed Coverity reported performance issues
Date Tue, 01 Jul 2014 13:48:55 GMT
I did review all, this was my only remark. You can push now, can you?

On Tue, Jul 1, 2014 at 3:44 PM, Santhosh Edukulla
<santhosh.edukulla@citrix.com> wrote:
> Sure, from next submission, will check on this. Please review other issues as well, it
will help,and so not to introduce regression issues, please push accordingly.
>
> Santhosh
> ________________________________________
> From: Daan Hoogland [daan.hoogland@gmail.com]
> Sent: Tuesday, July 01, 2014 9:38 AM
> To: Santhosh Edukulla
> Cc: dev@cloudstack.apache.org
> Subject: Re: Review Request 23194: Fixed Coverity reported performance issues
>
> I like the concise syntax, not demanding.
>
> On Tue, Jul 1, 2014 at 3:32 PM, Santhosh Edukulla
> <santhosh.edukulla@citrix.com> wrote:
>> Sorry, ver7 docs:
>> http://docs.oracle.com/javase/7/docs/api/java/util/Map.html
>>
>> Santhosh
>> ________________________________________
>> From: Santhosh Edukulla [santhosh.edukulla@citrix.com]
>> Sent: Tuesday, July 01, 2014 9:20 AM
>> To: Daan Hoogland
>> Cc: Abhinandan Prateek; cloudstack
>> Subject: RE: Review Request 23194: Fixed Coverity reported performance issues
>>
>> Daan,
>>
>> It seems equivalent, as per docs,
>>
>> "Copies all of the mappings from the specified map to this map (optional operation).
The effect of this call is equivalent to that of calling put(k, v) on this map once for each
mapping from key k to value v in the specified map. "
>>
>> http://docs.oracle.com/javase/8/docs/api/java/util/Map.html#putAll-java.util.Map-
>>
>> We can change if you still feels so.
>>
>> Regards,
>> Santhosh
>> ________________________________________
>> From: Daan Hoogland [daan.hoogland@gmail.com]
>> Sent: Tuesday, July 01, 2014 8:38 AM
>> To: Santhosh Edukulla
>> Cc: Abhinandan Prateek; cloudstack
>> Subject: Re: Review Request 23194: Fixed Coverity reported performance issues
>>
>> I think putAll is more efficient.
>>
>> On Tue, Jul 1, 2014 at 2:25 PM, Santhosh Edukulla
>> <santhosh.edukulla@citrix.com> wrote:
>>> Daan,
>>>
>>> You are added as reviewer, not sure why comments were disabled.
>>>
>>> Do you see it as an issue when used in its current form, considering original
issue is to minimize the lookups? Can be helpful with this way, down the lane if we are to
use entry for some other purpose? Assuming putall provides same efficiency as the current
form we have.
>>>
>>> Santhosh
>>> ________________________________________
>>> From: Daan Hoogland [daan.hoogland@gmail.com]
>>> Sent: Tuesday, July 01, 2014 8:13 AM
>>> To: Santhosh Edukulla
>>> Cc: Abhinandan Prateek; cloudstack
>>> Subject: Re: Review Request 23194: Fixed Coverity reported performance issues
>>>
>>> I can't seem to put a comment in this review request, hence a mail:
>>>
>>> why not use calls to putAll instead of the iteration over all elements? (only
valid for the first few iteration, where no further processing is done on the Entry)
>>>
>>>
>>> On Tue, Jul 1, 2014 at 8:59 AM, Santhosh Edukulla <santhosh.edukulla@citrix.com<mailto:santhosh.edukulla@citrix.com>>
wrote:
>>> This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23194/
>>>
>>> Review request for cloudstack, Abhinandan Prateek and daan Hoogland.
>>> By Santhosh Edukulla.
>>> Bugs: coverity<https://issues.apache.org/jira/browse/coverity>
>>> Repository: cloudstack-git
>>> Description
>>>
>>> Fixed Coverity reported performance issues like inefficient string concatenations,
wrong boxing or unboxing types, inefficent map element retrievals.
>>>
>>>
>>>
>>> Testing
>>>
>>> Built the code using simulator and deployed a datacenter
>>>
>>>
>>> Diffs
>>>
>>>   *   api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java
(68e9f94)
>>>   *   api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java
(d71ef03)
>>>   *   api/src/org/apache/cloudstack/context/CallContext.java (f29ae96)
>>>   *   core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java
(7bb6f5e)
>>>   *   engine/orchestration/src/com/cloud/agent/manager/AgentAttache.java (f11f69f)
>>>   *   plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
(b040633)
>>>   *   plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerPoolVms.java
(8042209)
>>>   *   plugins/network-elements/netscaler/src/com/cloud/network/element/NetscalerElement.java
(5199f60)
>>>   *   plugins/network-elements/netscaler/src/com/cloud/network/resource/NetscalerResource.java
(8c5aa1f)
>>>   *   plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LDAPConfigCmd.java
(619280d)
>>>   *   server/src/com/cloud/api/ApiResponseHelper.java (ed48d0b)
>>>   *   server/src/com/cloud/api/ApiServer.java (2ce6281)
>>>   *   server/src/com/cloud/api/dispatch/ParamProcessWorker.java (1592b93)
>>>   *   server/src/com/cloud/api/dispatch/ParamUnpackWorker.java (12e1226)
>>>   *   server/src/com/cloud/api/query/QueryManagerImpl.java (1182be5)
>>>   *   server/src/com/cloud/api/query/dao/TemplateJoinDaoImpl.java (80ef0f6)
>>>   *   server/src/com/cloud/configuration/ConfigurationManagerImpl.java (bb32c37)
>>>   *   server/src/com/cloud/network/NetworkServiceImpl.java (a574f10)
>>>   *   server/src/com/cloud/network/vpc/VpcManagerImpl.java (71f2316)
>>>   *   server/src/com/cloud/server/ConfigurationServerImpl.java (694f3cd)
>>>   *   server/src/com/cloud/server/StatsCollector.java (29ace93)
>>>   *   server/src/com/cloud/storage/VolumeApiServiceImpl.java (7af404e)
>>>   *   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java (71cf083)
>>>   *   server/src/com/cloud/template/TemplateAdapterBase.java (e2204da)
>>>   *   server/src/com/cloud/template/TemplateManagerImpl.java (694bd03)
>>>
>>> View Diff<https://reviews.apache.org/r/23194/diff/>
>>>
>>>
>>>
>>>
>>> --
>>> Daan
>>
>>
>>
>> --
>> Daan
>
>
>
> --
> Daan



-- 
Daan

Mime
View raw message