Change in ovirt-engine[master]: core: CommandsFactory initial cache size

Code Review gerrit at ovirt.org
Fri May 26 10:12:30 UTC 2017


>From Allon Mureinik <amureini at redhat.com>:

Allon Mureinik has submitted this change and it was merged.

Change subject: core: CommandsFactory initial cache size
......................................................................


core: CommandsFactory initial cache size

The initial cache size seems as though it was supposed to accommodate
all the possible command classes and thus prevent the map's
re-hashing at runtime.
While this seems like a noble cause, the calculation is just wrong,
for several reasons:

1. Despite what the generic signature may suggest, this map also
   contains query classes (see getQueryClass(String) for details).
   This issue will be addressed in subsequent patches.
2. Even if #1 wasn't true, this usage represents a misunderstanding
   of ConcurrentHashMap's API. The default load factor is 0.75, so
   even with the map initialized to this size, it's quite possible
   that it will have to be resized during the engine's lifetime.
3. Not all users have all features installed/enabled/used. E.g., many
   users may not be using Gluster, which has 45(!) commands. As every
   user will have quite a few potential commands that are never
   instantiated, the map is redundantly wide from the bat.
4. Setting point #2 aside, the calculation is still off by one, as
   VdcActionType.Unknown does not translate into a concrete command
   type.

This patch removes this premature, false, optimization and replaces
it with a straight forward creation of the map.

Change-Id: Iae31d70477c37a22bf3dd6215ced7c8649be8bdc
Signed-off-by: Allon Mureinik <amureini at redhat.com>
---
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandsFactory.java
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Tal Nisan: Looks good to me, approved
  Allon Mureinik: Verified; Passed CI tests



-- 
To view, visit https://gerrit.ovirt.org/77108
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iae31d70477c37a22bf3dd6215ced7c8649be8bdc
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Peřina <mperina at redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>


More information about the Engine-commits mailing list