Micro-optimisation.
model.isDir() does a full stat of a file.
We hover a lot, may as well use a cached version of the variable.
CCBUG: 379994
Details
Details
- Reviewers
mart
Diff Detail
Diff Detail
- Repository
- R119 Plasma Desktop
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage
Comment Actions
model.isDir() does a full stat of a file.
model.isDir() does indeed, but this uses model.isDir, which is a data role. The FolderView::data() implementation already uses a cache. The cache is inserted into the first time isDir() is called and evicted e.g. on item deletion. It normally shouldn't stat.
This makes me wary of this patch because I don't want us to adopt an anti-pattern of caching model data as Qt Quick var copies. I expect Qt Quick to be smart enough to optimize model access.
I assume you did this based on profiling? Maybe data() is broken?
Comment Actions
I would agree that we shouldn't be copying model data... But I'm not adding that, it's already there.
The FolderView::data() implementation already uses a cache
It seems we only cache the result of .desktop files that point to remote URLs. The rest goes into KDirModel.
A .desktop file to file:// stats every time.
Any non .desktop file goes to whatever KDirModel does.
FWIW, I'm working on the stuff here: 379994.
This doesn't actually fix anything there, the trace shows us doing an QFileInfo::isDir() but from a call to model.type
Comment Actions
Hmm shouldn't we improve the caching in either KDirModel or FolderModel then? Working around a performance issue in the model in delegate code just feels so wrong. Any potential other user of FolderModel (e.g. future Plasma Mobile file manager) would possibly have to know to do this too.