[senordhuff]

Hi there, thanks for the update and questions. I mentioned in my comment on 11th Oct 2019, 13:53 that we were also noticing the same issues with editByCell:false. We've been experimenting with both modes just trying to find something where the performance is acceptable. And, we are currently hoping that we can launch with editByCell:false. But, if we are forced to use editByCell:true to get decent performance, we may switch to that mode.

There is nothing intentional in the sample with the status_2_2 field. I do see that it is an selectItem with no value populated and the field next to it will display a PopUpTextAreaItem, so I can only speculate one of those things is causing the issue you describe but I don't see any issues myself when playing with that field.

Any idea when you might have some updates that have been committed that we can try?

[Isomorphic]

Hi there, thanks for the update and questions. I mentioned in my comment on 11th Oct 2019, 13:53 that we were also noticing the same issues with editByCell:false. We've been experimenting with both modes just trying to find something where the performance is acceptable. And, we are currently hoping that we can launch with editByCell:false. But, if we are forced to use editByCell:true to get decent performance, we may switch to that mode.

So with edit-by-row, can you clarify what performance issue you're hitting? Since the editor should not be dismissed during field navigation, several of the methods that could be potentially optimized in the hide/show editor chain don't run. Are you still seeing a lot of delay with field navigation in that situation - or are you talking about a delay when you complete the row edit or navigate between rows?

Any idea when you might have some updates that have been committed that we can try?

We were planning to commit some today, but from comment #14, the impression seemed to be that you had decided to proceed with some "hyperoptimization" patches (making assumptions beyond what the Framework in general can) tuned to your specific situation and were no longer waiting on anything immediate from us.

[senordhuff]

Hi, yes the major delay we are seeing is when completing the row edit and navigating between rows with editByCell:false

Yes, I have a series of patches but my hope was that your framework tuning would render my patches obsolete or unnecessary. I only wanted to use those patches in the event you guys weren't able to make any changes that had a positive impact on our performance issues.

[senordhuff]

Hi, I wanted to point out another performance improvement I found that you may want to consider. When a cell is edited and you exit the cell, storeUpdatedEditorValue() makes a call to setEditValue and does not set the suppressSummaryRecalc paramater. As a result, setEditValue is always calling calculateRecordSummaries which is a very expensive call. And, it is making this call even if the currently edited cell does not display summary values at all. These calls to calculateRecordSummaries are expensive and it is ok for our use case to simply set the suppressSummaryRecalc parameter to true. But, you may consider optimizing that logic to only call calculateRecordSummaries when absolutely necessary or make it configurable if certain use cases can tolerate skipping that summary calc to speed things up when editing.

[Isomorphic]

With regard to this thread, we'd like to limit "aggressive" optimizations to SC 12.1d, at least initially until we are confident that potential stability and regression risks are minimal. We didn't see a mention of which branch you're working with. Is it possible for you to test against that branch, or can you only work with the release?

[senordhuff]

Hi, we are using a recently nightly of 12.0. I can make plans to try this out with 12.1 soon and will keep you posted. I also plan to detail for you the "hyperoptimization" patches we have employed in the event any of them might make sense for the framework as a whole.

[Isomorphic]

Hi, I wanted to point out another performance improvement I found that you may want to consider. When a cell is edited and you exit the cell, storeUpdatedEditorValue() makes a call to setEditValue and does not set the suppressSummaryRecalc paramater. As a result, setEditValue is always calling calculateRecordSummaries which is a very expensive call. And, it is making this call even if the currently edited cell does not display summary values at all. These calls to calculateRecordSummaries are expensive and it is ok for our use case to simply set the suppressSummaryRecalc parameter to true. But, you may consider optimizing that logic to only call calculateRecordSummaries when absolutely necessary or make it configurable if certain use cases can tolerate skipping that summary calc to speed things up when editing.

The problem is that a field summary may depend not only on the field itself, but on the record - at least that's potentially true with user-defined summary functions. So we can't simply skip the update just because a field itself doesn't have a summary.

We'll have to discuss it internally, but that approach could only be used safely if no fields have custom summary functions, instead relying on the built-in SimpleType methods.

[senordhuff]

Hi, with regards to summaries, our preference is that whether or not summaries recalc in response to user edits would be configurable. In our case, we would be fine with skipping the recalc of summaries until the user saves the data.

[Isomorphic]

Looking at a performance profile of your sample, we do see in fact that most of the overhead seems to be due to summary processing and redrawing, and actually not that much due to the target of our current optimizations, the allowEditFormValueManipulation logic optimization. (Though of course, as you say, your real app behaves a bit differently.)

So it fact it probably makes sense to expose a property to allow summary updates during editing to be suppressed - but summaries would still be recalculated upon completing an edit row (or cell if editing by cell) unless ListGrid.autoSaveEdits is turned off.

[senordhuff]

Ok, I will plan to provide a summary of the patches we have implemented so you can comment. And, will be happy to try out that new potential property to suppress summary updates when you let me know it is available.

[senordhuff]

Hi there, here is a summary of the optimizations that we've applied to our environment.

1. For the cancelEditing, discardEdits, and discardAllEdits methods, if summaries are enabled, turn them off before calling these methods and then turn them back on to avoid expense summary calculations. We are handling the summary recalculations with our own code.
2. Setting allowEditFormValueManipulation to false
3. Setting canChangeNonFieldValues to false on all DynamicForms and ListGrids
4. In isc_ListGrid__editItemsDrawingNotification, only include items that are visible and editable in the first loop
5. In isc_ListGrid__showEditForm, only process items that are editable when looping over drawn fields
6. In isc_ListGrid_makeEditForm, only set record and editedRecord once and not on each iteration. Also, do not process items in the loop if you can't edit the cell
7. In isc_ListGrid_recordHasChanges, do not check all datasource fields
8. In getEditDisplayValues, only loop over and process editable fields
9. In storeUpdatedEditorValue, when making the first call to setEditValue(), explicitly set suppressSummaryRecalc param to true
10. In updateEditRow, only include editable items and their associated values

Hopefully that is helpful as you are reviewing potential optimizations for the framework as a whole. We'd strongly prefer not to keep using all of these custom optimizations if there are decent candidates for optimizing at the framework level or providing configuration flags to enable these.

[Isomorphic]

Did you say you were able to test against SC 12.1d? Last week optimizations to speed up ListGrid record comparison were added, and yesterday we added a new property ListGrid.updateSummariesDuringEditing to allow summary updates to be disabled during editing. We also improved how the logic for the undocumented property allowEditFormValueManipulation works (so it need not be switched off).

[senordhuff]

Hi, I haven't yet tested with 12.1. We are focused on getting our release out that includes all of the optimizations we applied. After that is out and in production, I will plan to take a look at what you've done with 12.1. Thank you for the update and for adding the new property and I will provide an update as soon as possible.