Enhancement: minimize calls to reloadData on CPTableView #457

Closed
klaaspieter opened this Issue Feb 9, 2010 · 12 comments

Comments

Projects
None yet
4 participants
Contributor

klaaspieter commented Feb 9, 2010

Currently the tableview calls reload data on itself after a drag and when calling reloadColumns:rows:.
After checking the NSTableView I think CPTableView should only recall reloadData on itself when the datasource is set, after that it should always reload the minimum number of columns / rows.

The current reloadData behavior makes it really hard to implement variable sized rows for large tables.

Contributor

klaaspieter commented Jan 5, 2011

This isn't as much of a problem as I thought. CPTableView will always attempt to reload the minimal number of rows. Unlike NSTableView which will reload all rows when reloadData is called.

The advantage to our approach is that the tableview is always fast even when loading it initially. The advantage to their method is that NSTableView always has all the information it needs available from the start. It would be very hard for us to implement double click to auto-size column because we don't have the required row information available.

Contributor

aparajita commented Apr 16, 2012

I think the larger issue is that reloadDataForRowIndexes:columnIndexes: is not really implemented, it just calls reloadData.

Contributor

klaaspieter commented Apr 16, 2012

The complexity of implementing is not worth the actual speed gain. In Cocoa's implementation reloadData reloads all the columns and rows. Our implementation only reloads the visual ones. Our implementation of reloadDataForRowIndexes:columnIndexes: would probably also only reload visible columns and rows that are in the index sets. The number of rows visible at one time in a tableview is usually so small that the difference in performance between reloading all or just a subset of visual rows and columns should be negligible.

Contributor

aparajita commented Apr 16, 2012

It is totally possible that the user will want to update a single cell. In that case it is a huge waste to reload everything. I actually have an implementation of this somewhere, it isn't that hard.

cappbot commented May 9, 2012

Milestone: 1.0. Labels: #accepted, #needs-patch, AppKit, feature. What's next? This issue needs a volunteer to write and submit code to address it.

Contributor

daboe01 commented Jun 26, 2014

hi aparajita,
do you have your implementation handy?
could you post it here?
someone may be using it to make a PR.
best greetings,
daniel

Contributor

aparajita commented Jun 26, 2014

@daboe01 In the golden future I will be doing a lot of work on CPTableView, including this issue. But I am not in a position to do anything about this now.

Contributor

daboe01 commented Jul 29, 2014

seems to be fixed by PR #1892

Contributor

daboe01 commented Jul 7, 2016

i am closing this because the current implementation only reloads visible cells. There will be not measurable speed gain from constraining any further.
#+wont-fix

Contributor

daboe01 commented Aug 14, 2016

+#wont-fix

cappbot commented Aug 14, 2016

Milestone: 1.0. Labels: #wont-fix, AppKit, feature. What's next? A reviewer or core team member has decided against acting upon this issue.

cappbot commented Aug 14, 2016

Milestone: 1.0. Labels: #wont-fix, AppKit, feature. What's next? A reviewer or core team member has decided against acting upon this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment