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.eduku...@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.hoogl...@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.eduku...@citrix.com> wrote: >> Sorry, ver7 docs: >> http://docs.oracle.com/javase/7/docs/api/java/util/Map.html >> >> Santhosh >> ________________________________________ >> From: Santhosh Edukulla [santhosh.eduku...@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.hoogl...@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.eduku...@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.hoogl...@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.eduku...@citrix.com<mailto:santhosh.eduku...@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
