ICEfaces
  1. ICEfaces
  2. ICE-5704

Javascript tree update optimisations

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.2-EE-GA_P01, 1.8.2a
    • Fix Version/s: 1.8.3, 1.8.2-EE-GA_P02, 2.0.0
    • Component/s: Bridge
    • Labels:
      None
    • Environment:
      IE 8, FF 3.5.9, Win XP SP3

      Description

      Have an example application with thousands of nodes in a tree. Actually 4 trees, parallel to each other, creating the impression of a tree table, with each tree being a different column. The leftmost tree has drag and drop setup on each node, as well as a context menu.

      Expanding and unexpanding folder nodes was noticeably slow, and dragging nodes between folders was noticeably slow, and dragging nodes from or to the root was very slow.

        Activity

        Hide
        Mark Collette added a comment -

        Ran the following scenarios in IE 8's javascript profiler, finding large time durations, and figured out ways to optimise the code, to reduce the time durations:

        • Expand folder
        • Unexpand folder
        • Drag node from folder, and drop it into another folder
        • Drag node from root and drop it into folder
        • Drag node from folder and drop it into root

        These are the optimisations I made:

        • In application.js, the updates command dispatcher, for every updated element, was invoking Ice.StateMon.checkAll(). Made it do it once, after all the updates.
        • Ice.StateMon.checkAll() determines if any monitors for drag, drop, autocomplete have been updated or removed. It used the $ form of document.getElementById twice per monitor to do so. I changed it to do the lookup once and reuse the result.
        • Droppables.remove(element) prunes all (usually just the one) element from its drops list by comparing element references, which involved re-looking-up the element to be removed with the $ form of document.getElementById, for every element in the drops list. Made it do the lookup once, before iterating through the list, and introduces a variant that compared the id, if it was specified.
        • Attempted to simplify the javascript rendered by by the panelGroup for being a drop target. Droppables.add is called in the onmouseover handler as well as there being a script tag inside the panelGroup's div that explicitly calls Droppables.add. Tried removing the onmouseover, since I figured the script tag always executes. But that made drag and drop really glitchy, so I reverted it.
        • In element.js, for each implementation of the Element class, there are different implementations of updateDOM(update), which indirectly calls down to disconnectEventListeners(), which needs to check the optimizedJSListenerCleanup property in the bridge. That meant that for every updated element, there was a DOM traversal to find the bridge element. Pulled the lookup for that property all the way out of updateDOM(update), out of the element loop, so it would be found once, and then passed that into the functions.
        Show
        Mark Collette added a comment - Ran the following scenarios in IE 8's javascript profiler, finding large time durations, and figured out ways to optimise the code, to reduce the time durations: Expand folder Unexpand folder Drag node from folder, and drop it into another folder Drag node from root and drop it into folder Drag node from folder and drop it into root These are the optimisations I made: In application.js, the updates command dispatcher, for every updated element, was invoking Ice.StateMon.checkAll(). Made it do it once, after all the updates. Ice.StateMon.checkAll() determines if any monitors for drag, drop, autocomplete have been updated or removed. It used the $ form of document.getElementById twice per monitor to do so. I changed it to do the lookup once and reuse the result. Droppables.remove(element) prunes all (usually just the one) element from its drops list by comparing element references, which involved re-looking-up the element to be removed with the $ form of document.getElementById , for every element in the drops list. Made it do the lookup once, before iterating through the list, and introduces a variant that compared the id, if it was specified. Attempted to simplify the javascript rendered by by the panelGroup for being a drop target. Droppables.add is called in the onmouseover handler as well as there being a script tag inside the panelGroup's div that explicitly calls Droppables.add . Tried removing the onmouseover, since I figured the script tag always executes. But that made drag and drop really glitchy, so I reverted it. In element.js, for each implementation of the Element class, there are different implementations of updateDOM(update), which indirectly calls down to disconnectEventListeners(), which needs to check the optimizedJSListenerCleanup property in the bridge. That meant that for every updated element, there was a DOM traversal to find the bridge element. Pulled the lookup for that property all the way out of updateDOM(update), out of the element loop, so it would be found once, and then passed that into the functions.
        Hide
        Mark Collette added a comment -

        Discussed with Mircea about a future optimisation of halving the element lookups in the updates processing. In application.js, the updates command dispatcher, for every updated element, does a document.getElementById() before and after applying the updates. But it's probably possible to have updateDOM(update) implementations return the new element object, instead of having to look it up, since they either created the new element, or instead kept the same element and just updated the innards.

        Show
        Mark Collette added a comment - Discussed with Mircea about a future optimisation of halving the element lookups in the updates processing. In application.js, the updates command dispatcher, for every updated element, does a document.getElementById() before and after applying the updates. But it's probably possible to have updateDOM(update) implementations return the new element object, instead of having to look it up, since they either created the new element, or instead kept the same element and just updated the innards.
        Hide
        Mark Collette added a comment -

        Committed to trunk the initial set of optimisations implemented so far.

        Subversion 21421

        Show
        Mark Collette added a comment - Committed to trunk the initial set of optimisations implemented so far. Subversion 21421
        Hide
        Mark Collette added a comment -

        Any theorised potential future optimisations will be dropped, and this jira marked resolved. This is all already in icefaces trunk. Verify that it's in icefaces2 trunk, and target this also to ICEfaces 2 Beta2.

        Show
        Mark Collette added a comment - Any theorised potential future optimisations will be dropped, and this jira marked resolved. This is all already in icefaces trunk. Verify that it's in icefaces2 trunk, and target this also to ICEfaces 2 Beta2.
        Hide
        Mark Collette added a comment -

        These changes do not exist in icefaces2, and would have to be ported over. Not all would apply, as some were specific to to our DOM updating. But the drag and drop ones would still apply.

        Show
        Mark Collette added a comment - These changes do not exist in icefaces2, and would have to be ported over. Not all would apply, as some were specific to to our DOM updating. But the drag and drop ones would still apply.
        Hide
        Mark Collette added a comment -

        Adopted the drag and drop specific optimisations into the icefaces2 compat codebase. The bridge optimisations specific to dom updates were not applicable.

        icefaces 2 trunk
        Subversion 23136

        Show
        Mark Collette added a comment - Adopted the drag and drop specific optimisations into the icefaces2 compat codebase. The bridge optimisations specific to dom updates were not applicable. icefaces 2 trunk Subversion 23136
        Hide
        Isuru Perera added a comment -

        I have an issue with ICEfaces EE 1.8.2.GA_P02

        It may not be related to this JIRA issue. Please check following forum post.

        http://www.icefaces.org/JForum/posts/list/0/18206.page#66225

        Thanks.

        Show
        Isuru Perera added a comment - I have an issue with ICEfaces EE 1.8.2.GA_P02 It may not be related to this JIRA issue. Please check following forum post. http://www.icefaces.org/JForum/posts/list/0/18206.page#66225 Thanks.

          People

          • Assignee:
            Mark Collette
            Reporter:
            Mark Collette
          • Votes:
            5 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: