Monday, June 29, 2015

SlickGrid Async Post Render Cleanup

This SlickGrid issue has been a bit of a hot potato.

Discussions at:
  github.com/mleibman/SlickGrid/issues/82
  github.com/mleibman/SlickGrid/issues/855
in particular.

So MLeibman has gone on record as saying that using jQuery data and event handlers in AsyncPostRender nodes is not a good idea and that allowing a cleanup might give people the idea that it's OK, which it really isn't.
The counter argument run along the lines that jQuery is used more and more for event and data binding, and that the amount of binding is a continuum - it may be used only lightly, all the way to fully fledged jQueryUI plugins, thus there may be a valid use case, and if so, having provided the means to implement it the grid should provide the means to clean up.

Having spent some time maintaining the SubSonic micro-ORM, one of the things I liked most about the project was the philosophy that the developer is in the end responsible for their decisions, and it is the place of the framework developer to simply offer as wide a range of options as possible - if necessary, also opening up the possibility of these options being abused.
So in this spirit, I decided to try to work out a flexible cleanup method for async post rendered nodes that would also work asynchronously, just like the post render method itself.

After examination of the code, and an initial attempt to code a solution, it became clear that there were three cleanup scenarios needing handling (remembering that one or more cells in any row might have been post rendered):
  1. deletion of an entire row from the DOM and grid cache
  2. deletion of a single cell from the DOM and grid cache
  3. the re-rendering of a previously rendered cell without cell deletion
In addition, the SlickGrid code has a workaround for a Mac scrolling issue that delays deletion of the grid node where a mouse scroll started (using var zombieRowNodeFromLastMouseWheelEvent). This is a row cleanup like the regular one, but it is handled in a different section of code and might easily escape notice. I'd just about bet that was what was causing Jonozzz's small remaining memory leak in Issue 82.

The first step was to add the example example10a-async-post-render-cleanup.html to the examples folder of my alternative master repository.

The modifications to SlickGrid added two new options to the Grid:
    enableAsyncPostRenderCleanup (bool)
    asyncPostRenderCleanupDelay (int)
and an asyncPostRenderCleanup (function) option to each column definition.
A cleanupBeforeRender parameter was added to the end of the AsyncPostRender function arguments.

Internally, where nodes were cleaned up using direct DOM manipulation previously, the code now checks to see if post render cleanup is enabled, and if the row or cell had been post rendered. If so, any post rendered cells are added to a cleanup queue, followed by the row node in the case of row deletion.
Any process that generates queue events afterwards kicks off the post render cleanup on timed delay.
A cleanup queue push example:
postProcessedCleanupQueue.push({
    actionType: 'C',
    groupId: postProcessgroupId,
    node: cacheEntry.cellNodesByColumnIdx[ columnIdx ],
    columnIdx: columnIdx | 0,
    rowIdx: rowIdx
  });
ActionType is C to clean up a post rendered cell, and R to delete a parent row node once the child cells have been cleaned up.GroupId groups cleanup records so we can clean up a row's worth of cells at a time (the same way they are post rendered). The cleanup function processes queue entries matching the GroupId of the first queue entry, then re-calls itself after a delay of asyncPostRenderCleanupDelay ms.
The node is the actual cell or row DOM node, and the column and row indexes may be used to reference the column or row data.

So case 1 (row deletion) is handled by adding any post-processed column nodes to the queue, then the row node.
Case 2 is handled by adding the cell to the queue if it has been post-processed.
Case 3 is handled by including a parameter cleanupBeforeRender (bool) in the PostRender call. If true, this indicates that the cell has been post-rendered already and is being re-rendered without deleting the cell, and that the render function should therefore clean up the previous actions prior to starting the new ones.

So as a summary, here are the key parts of the example page demonstration AsyncPostRender with cleanup:
var options = {
    ... ,
    enableAsyncPostRender: true,
    asyncPostRenderDelay: 50,
    enableAsyncPostRenderCleanup: true,
    asyncPostRenderCleanupDelay: 40
  };

  var columns = [
    ... ,
    {id: "chart", name: "Chart", formatter: waitingFormatter, rerenderOnResize: true,
        asyncPostRender: renderSparkline, asyncPostRenderCleanup: cleanupSparkline}
  ];

  function renderSparkline(cellNode, rowIdx, dataContext, colDef, cleanupBeforeRender) {
    ...
  }
  
  function cleanupSparkline(cellNode, rowIdx, colDef) {
    ...
  }

This provides a comprehensive API and async cleanup process, which should be able to handle cleanup gracefully, as long as the weight of the render and cleanup is not so great that it compromises the performance of the basic grid code.
I reiterate: it is up to the developer to plan, test and monitor the performance of the grid in conjunction with 'heavy' external controls under production conditions. It is easy to get into trouble. Use this tool wisely.

See the commit for details, and check out the sample page for usage notes.

No comments:

Post a Comment