Announcement

Collapse
No announcement yet.
X
  • Filter
  • Time
Clear All
new posts

  • senordhuff
    replied
    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.

    Leave a comment:


  • Isomorphic
    replied
    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).

    Leave a comment:


  • senordhuff
    replied
    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.

    Leave a comment:


  • senordhuff
    replied
    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.

    Leave a comment:


  • Isomorphic
    replied
    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.

    Leave a comment:


  • senordhuff
    replied
    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.

    Leave a comment:


  • Isomorphic
    replied
    Originally posted by senordhuff View Post
    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.

    Leave a comment:


  • senordhuff
    replied
    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.

    Leave a comment:


  • Isomorphic
    replied
    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?

    Leave a comment:


  • senordhuff
    replied
    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.

    Leave a comment:


  • senordhuff
    replied
    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.

    Leave a comment:


  • Isomorphic
    replied
    Originally posted by senordhuff View Post
    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.


    Leave a comment:


  • senordhuff
    replied
    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?

    Leave a comment:


  • Isomorphic
    replied
    While looking at your sample in the context of our Framework optimizations, we noticed that it's not configured with editByCell:true. Obviously, that means the logic path during field navigation is different, rendering some optimizations moot. Did you settle on edit-by-row instead?

    With your current sample, we noticed that during field navigation, the editor may exit when crossting the "Status_2_2" field. Is this something intentional, or should we look into it?

    When running comparisons, we're not able to only look at canEdit:true fields (which have formItems) in our optimizations as we want to capture programmatic changes as well to any field. So our changes may not duplicate what you've done, but hopefully they'll minimize any required patching.

    Leave a comment:


  • senordhuff
    replied
    Thank you for the update. I ended up creating a series of patches to address many of our performance issues in the methods I have highlighted in this thread. I am happy to share that with you if you want to see it. I'm sure that some of my patches would not be acceptable for all use cases. One of the biggest improvements I saw was eliminating some instances where the native logic iterates over all properties on a record because we do add a variety of objects and properties to our records that would never be edited by a user. That is a patch we made to getEditDisplayValues() for example. I don't think my sample adequately captures the variety of properties and objects we hang onto the record and that is why the performance impact wasn't quite as dramatic in my sample. In our application, it was slowing down things dramatically to iterate over all the properties and objects. Performance is still slow with my patches but it is in the range of acceptable. Hopefully, your planned improvements will help further.

    Also, just to clarify we have 800+ columns on the grid AND on the datasource. So, a typical use case is to expose a view with 10 editable visible fields, 30 total visible fields, 800+ total fields including those that are hidden from the current view, and 800+ datasource fields. This occurs because we dynamically add many fields at run-time based on some child objects. The user will edit these child objects in a "detail" view and then we display a series of "flattened" columns representing those child objects in this main grid.

    Thanks again for your help with these issues and let me know if you need any more feedback.

    Leave a comment:

Working...
X