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
