Java Collection for Optimal performance

Question

I have below snippet of code which gets all the available menu items for a zone(top or left)and adds the widgets for which user has access to an ArrayList collection. For now the getWidgetsByZone(zone) returns 64 items and iterates over them. Am seeing some performance lag in this method(in a tool called GlowRoot which logs the time trace for each user action) I dont know why. Am trying to improve the performance by switching to other optimal collection. Can someone help me to choose the optimal collection for my scenario?

AM using JDK 7, Hibernate 3.6 and Spring 3.1

Here is DashboardService.getMenuItems() implementation

public List<IContentWidget> getMenuItems(String zone) {
        List<IContentWidget> widgets = new ArrayList<IContentWidget>();
        if (zone != null) {

            List<IPersistentEntityInstance> instances = getWidgetsByZone(zone);

            for (IPersistentEntityInstance instance : instances) {
                IContentWidget contentWidget = (IContentWidget) instance;
                if (contentWidget.getZones() == null) continue;

                // block widgets that should only show up in mobile / responsive ui
                if (contentWidget.getZones().contains(RESPONSIVE_VISIBLE)) continue;

                // Add widget only if the current user has read permission on the entity.
                if (contentWidget.getTargetItemScreen() != null || contentWidget.getTargetListScreen() != null) {
                    if (isAccessible(contentWidget)) {
                        widgets.add(contentWidget);
                    }
                }
                else {
                    widgets.add(contentWidget);
                }
            }
        }
        Collections.sort(widgets, new Comparator<IContentWidget>() {

            public int compare(IContentWidget o1, IContentWidget o2) {
                int i = o1.getOrderNum() - o2.getOrderNum();
                return i == 0 ? 0 : i < 0 ? -1 : 1;
            }

        });
        return widgets;
    }

Implementation of DashboardService.isAccesible()

private boolean isAccessible(IContentWidget contentWidget) {
    boolean isWidgetAccessible = false;
    String permission = contentWidget.getDisplayPermission();
    if (permission != null) {
        isWidgetAccessible = authorizationService.userHasPermission(SecurityHelper.getAuthenticatedUser(),
                permission);
    }
    else {
        IBaseScreen screen = contentWidget.getTargetItemScreen() == null ? contentWidget.getTargetListScreen()
                : contentWidget.getTargetItemScreen();
        // return true when target screen is 'null', this means that target link cannot be secured because it is not
        // associated with any entity
        if (screen == null) {
            isWidgetAccessible = true;
        }
        else {
            IAccessEntry access = authorizationService.getAccessForEntityMetadata(screen.getEntityMetadata());

            // fetching metadata from entityMetadataService again to trigger population of facade
            if (screen instanceof IListScreen && access.getIsReadable()) {
                isWidgetAccessible = true;
            }
            else if (screen instanceof IItemScreen && access.getIsCreatable()) {
                isWidgetAccessible = true;
            }
        }
    }
    return isWidgetAccessible;
}

Implementation of getWidgetsByZone method

public List<IPersistentEntityInstance> getWidgetsByZone(String zone) {
    IEntityMetadata entity =  entityService.findEntityMetadataByName(ContentWidget.class.getSimpleName());
return entityService.runNamedQuery(entity, NamedQueryList.WIDGETS_BY_ZONE, new Object[] { zone });
    }

Here is my ContentWidget Entity

@LocalOnly
@Entity
@EntityMetadataDefaults(editable = false)
@Audited
@NamedQueries({
    @NamedQuery(name = NamedQueryList.DASHBOARD_WIDGETS, query = "from ContentWidget where zones like '%dashboard%' and dashboardContexts.size = 0 order by orderNum", hints = {
            @QueryHint(name = "org.hibernate.cacheable", value = "true"),
            @QueryHint(name = "org.hibernate.cacheRegion", value = "Metadata") }),
    @NamedQuery(name = NamedQueryList.WIDGETS_BY_ZONE, query = "from ContentWidget where zones like '%' || ? || '%' order by orderNum", hints = {
            @QueryHint(name = "org.hibernate.cacheable", value = "true"),
            @QueryHint(name = "org.hibernate.cacheRegion", value = "Metadata") }),
    @NamedQuery(name = NamedQueryList.WIDGETS_BY_ZONE_ORDER_BY_NAME, query = "from ContentWidget where zones like '%' || ? || '%' order by displayName", hints = {
            @QueryHint(name = "org.hibernate.cacheable", value = "true"),
            @QueryHint(name = "org.hibernate.cacheRegion", value = "Metadata") }),
    @NamedQuery(name = NamedQueryList.WIDGETS_BY_DASHBOARD_URL, query = "from ContentWidget where dashboardUrl like ? || '%'", hints = {
            @QueryHint(name = "org.hibernate.cacheable", value = "true"),
            @QueryHint(name = "org.hibernate.cacheRegion", value = "Metadata") }),
    @NamedQuery(name = NamedQueryList.WIDGETS_BY_NAME, query = "from ContentWidget where name = ?", hints = {
            @QueryHint(name = "org.hibernate.cacheable", value = "true"),
            @QueryHint(name = "org.hibernate.cacheRegion", value = "Metadata") }),
    @NamedQuery(name = NamedQueryList.WIDGETS_BY_CONTEXT, query = "from ContentWidget where zones like '%dashboard%' and ? in elements(dashboardContexts) order by orderNum", hints = {
            @QueryHint(name = "org.hibernate.cacheable", value = "true"),
            @QueryHint(name = "org.hibernate.cacheRegion", value = "Metadata") }) })
@Cacheable
@Cache(usage = CacheConcurrencyStrategy.READ_WRITE, region = "Metadata")
@EventDefaults({
        @EventHandlerDefaults(beanName = "contentWidgetPermissionCommand", eventType = EventTypeEnum.OnBeforeCreate),
        @EventHandlerDefaults(beanName = "contentWidgetPermissionCommand", eventType = EventTypeEnum.OnBeforeUpdate) })
@ReadPermission(name = AuthorizationService.ANONYMOUS_PERMISSION)
public class ContentWidget implements IContentWidget {
    private static final long       serialVersionUID = 1680304771254400928L;
    private String                  packageGuid;
    private Long                    id;

    private String                  name;                                   // unique name to reconcile imported data
    private String                  displayName;                            // used by UI
    private String                  description;                            // anchor title attribute
    private int                     orderNum;                               // universal ordering
    private String                  zones;                                  // csv: "top,left,dashboard,context,..."
    private String                  iconClass;
    private String                  displayPermission;

    // menu settings
    private IContentWidget          parent;

    private String                  targetUrl;

    private IListScreen             targetListScreen;
    private IItemScreen             targetItemScreen;
    private boolean                 isCacheable;
    private boolean                 isDivider;

    private boolean                 isPopup;

    private List<IEntityMetadata>   contextEntities;                        // for contextual menus
    protected IFilterDefinition     entityFilterDefinition;
    // dashboard settings
    private int                     dashboardWidth   = 1;
    private String                  dashboardUrl;
    private String                  dashboardWidgetType;
    private IListScreen             dashboardListScreen;
    private IItemScreen             dashboardItemScreen;
    private List<IEntityMetadata>   dashboardContexts;                      // for item screen dashboards

    private ISessionService         sessionService;
    @Autowired
    private IPassportContextService passportContextService;
    @Autowired
    private IReportingConfiguration reportingConfiguration;
    private Timestamp               createdAt;
    private Timestamp               updatedAt;
    private ICustomNamedQuery       menuCountQuery;
    private Set<IPassportContext>   passportContexts;
    }

Performance trace Update:

The method performance trace in GlowRoot is as below

60.0% com.dc.core.presentation.presenter.impl.WebContentPresenter.getMenuHTML(WebContentPresenter.java:435)
50.0% com.dc.core.presentation.service.impl.DashboardService.getMenuItems(DashboardService.java:258)
30.0% com.dc.core.presentation.service.impl.DashboardService.isAccessible(DashboardService.java:382)

Here is my WebContentPresenter.getMenuHTML() implementation

public String getMenuHTML(String baseUrl, String zone, String cssClass, IPersistentEntityInstance entityInstance) {
    (line 435) List<IContentWidget> instances = dashboardService.getMenuItems(zone);
    StringBuffer html = new StringBuffer();

    if (instances == null || instances.isEmpty()) {
        html.append("&nbsp;");
    }
    else {
        Map<Long, List<IContentWidget>> treeData = new HashMap<Long, List<IContentWidget>>();
        for (IContentWidget instance : instances) {
            BeanWrapperImpl bean = new BeanWrapperImpl(instance);
            Object parent = bean.getPropertyValue("parent");
            Long parentId = -1L;
            if (passportContextService.getIsInContext(instance)) {
                if (parent != null) {
                    parentId = ((IContentWidget) parent).getId();
                }
                List<IContentWidget> children = treeData.get(parentId);
                if (children == null) {
                    children = new ArrayList<IContentWidget>();
                }
                children.add(instance);
                treeData.put(parentId, children);
            }
        }

        generateTreeHtml(html, treeData, -1L, baseUrl, "parent", entityInstance,
                authorizationService.userHasAdminPermission(SecurityHelper.getAuthenticatedUser()));
    }
    return String.format("<ul class=\"%s\">%s</ul>", cssClass, html.toString());
}

Why call sort in the end? Can the DB query for getWidgetsByZone() method not return results already sorted? — SJha, Sep 22 '14 at 20:34
A `Comparator` only mandates that you return a negative number, a positive number, or zero for less than, greater than, or equal to. You don't have to do the ternary in your `return` statement, since it'd be covered by the difference of the two numbers anyway. — Makoto, Sep 22 '14 at 20:34
This question appears to be off-topic because it is asking for code review. — Boris the Spider, Sep 22 '14 at 20:34
What do some of the other operations entail? Like, what type does `getZones()` return? Is that `contains(RESPONSIVE_VISIBLE)` a O(n) operation, and if so, how big is n? What does `isAccessible` do? — David Conrad, Sep 22 '14 at 20:40
Also, what does "some performance lag" mean? Exactly what kind of performance are you seeing? Have you tried to narrow down which part of the code is taking the time? — David Conrad, Sep 22 '14 at 20:42
@SaketJha Nope, DB query for getWidgetsByZone() method does not not return sorted results — OTUser, Sep 22 '14 at 20:43
Any reason it can't be made to, by adding an `ORDER BY` clause to it? However, this is premature, since we don't know if the sort is the performance problem. — David Conrad, Sep 22 '14 at 20:48
@RanPaul Are you using Hibernate / some other ORM in this? Is getWidgetsByZone() fetching data from multiple tables and if so are some of the entities lazily fetched? I am wondering if internally individual DB queries are issued when iterating over List instances ? A join fetch can help in this scenario. — SJha, Sep 22 '14 at 20:48
@SaketJha yes am using Hibernate 3.6 and Spring 3.1. getWidgetsByZone() fetching data from single table — OTUser, Sep 22 '14 at 20:54
@DavidConrad There is performance lag in jdbc statements and in code. Am working on fixing code performance lag — OTUser, Sep 22 '14 at 20:56
Yes, but **where** is the code performance lag? What makes you think it has anything to do with `ArrayList`? Have you tried to identify *which parts* of this method are taking the time? — David Conrad, Sep 22 '14 at 21:02
@DavidConrad updated question, please check at Performance trace Update: — OTUser, Sep 22 '14 at 21:12

score 2 · Answer 1 · answered Sep 22 '14 at 20:42

For 64 items difference between collections is not significant. I would rather investigate IContentWidget methods calls. How do you get those instances? Maybe each time you call getter, there is performed query to database? Could you provide some more details about persistence layer?

StackFlowed · Accepted Answer · 2014-09-22T20:52:42.303

For code readability I prefer :

public List<IContentWidget> getMenuItems(String zone) {
    if(zone == null){
        return Collections. < IContentWidget > emptyList();
    }
    List<IContentWidget> widgets = new ArrayList<IContentWidget>();
    List<IPersistentEntityInstance> instances = getWidgetsByZone(zone);
    for (IPersistentEntityInstance instance : instances) {
        IContentWidget contentWidget = (IContentWidget) instance;
        if (contentWidget.getZones() == null || contentWidget.getZones().contains(RESPONSIVE_VISIBLE)) {
           continue;
        }
        // Add widget only if the current user has read permission on the entity.
        if (contentWidget.getTargetItemScreen() == null ||
            contentWidget.getTargetListScreen()== null) {
            widgets.add(contentWidget);continue;
        }
        if (isAccessible(contentWidget)) {
             widgets.add(contentWidget);
        }


    }
}
Collections.sort(widgets, new Comparator<IContentWidget>() {

    public int compare(IContentWidget o1, IContentWidget o2) {
        return o1.getOrderNum() - o2.getOrderNum();
    }

});
return widgets;
}

Also change

private boolean isAccessible(IContentWidget contentWidget) {
    boolean isWidgetAccessible = false;
    String permission = contentWidget.getDisplayPermission();
    if (permission != null) {
        return authorizationService.userHasPermission(SecurityHelper.getAuthenticatedUser(),
                permission);
    }
    else {
        IBaseScreen screen = contentWidget.getTargetItemScreen() == null ? contentWidget.getTargetListScreen()
                : contentWidget.getTargetItemScreen();
        // return true when target screen is 'null', this means that target link cannot be secured because it is not
        // associated with any entity
        if (screen == null) {
            return true;
        }
        else {
            IAccessEntry access = authorizationService.getAccessForEntityMetadata(screen.getEntityMetadata());

            // fetching metadata from entityMetadataService again to trigger population of facade
            if (screen instanceof IListScreen && access.getIsReadable()) {
                isWidgetAccessible = true;
            }
            else if (screen instanceof IItemScreen && access.getIsCreatable()) {
                isWidgetAccessible = true;
            }
        }
    }
    return isWidgetAccessible;
}

Java Collection for Optimal performance

2 Answers2