Differential D13135

Introduce Storage::articlesForCache and more lazy-loading
Needs ReviewPublic
Actions

Authored by pinaraf on May 26 2018, 11:05 AM.

Details

Reviewers
krop
mlaurent
Summary
  • Further delay loading articles on fetch
  • New call, articlesForCache, returning (guid, status, hash, title)[]
  • Use that call in Feed
  • Introduce a clearCache method in TreeNode to drastically reduce memory usage
  • Fix a regression from previous lazy-loading patches

Diff Detail

Branch
lazyLoading
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18405
Build 18423: arc lint + arc unit
pinaraf created this revision.May 26 2018, 11:05 AM
Restricted Application added a project: KDE PIM. · View Herald TranscriptMay 26 2018, 11:05 AM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
pinaraf requested review of this revision.May 26 2018, 11:05 AM
pinaraf edited the summary of this revision. (Show Details)May 26 2018, 11:10 AM
pinaraf added a reviewer: krop.
pinaraf updated this revision to Diff 34918.May 26 2018, 11:15 AM
  • Remove wrong comment
mlaurent added a subscriber: mlaurent.May 28 2018, 6:56 AM
mlaurent added inline comments.
interfaces/feedstorage.h
45 ↗(On Diff #34918)

const QDateTime &

src/article.cpp
266 ↗(On Diff #34918)

why not assign d directly here ?

src/dummystorage/feedstoragedummyimpl.cpp
125 ↗(On Diff #34918)

const QString & ?

src/feed/feed.cpp
264

const ... & ?

pinaraf updated this revision to Diff 35966.Jun 10 2018, 8:49 PM
  • Fix segfault when adding feed
mlaurent added inline comments.Jun 13 2018, 6:21 AM
interfaces/feedstorage.h
45 ↗(On Diff #34918)

still not done

plugins/mk4storage/feedstoragemk4impl.cpp
207 ↗(On Diff #35966)

const int

src/article.cpp
266 ↗(On Diff #34918)

why not assign d directly here ?

src/dummystorage/feedstoragedummyimpl.cpp
125 ↗(On Diff #34918)

still not done

src/feed/feed.cpp
264

still not done

pinaraf updated this revision to Diff 69015.Oct 29 2019, 10:20 PM
  • Further delay loading articles on fetch
  • Introduce new Storage::articlesForCache
pinaraf marked 9 inline comments as done.Oct 29 2019, 10:22 PM

All your comments have been taken into account, branch cleaned, rebased on top of origin/master. And it has been dog-fed for more than a year… (so sorry for the delay)

mlaurent requested changes to this revision.Oct 31 2019, 8:34 AM
mlaurent added inline comments.
src/feed/feed.cpp
263–265

const QVector ...

787–788

Remove it as it's commented

924–927

coding style { ... }

928

same coding style { ... }

This revision now requires changes to proceed.Oct 31 2019, 8:34 AM
pinaraf updated this revision to Diff 69134.Oct 31 2019, 7:40 PM
  • It's flying fine
  • Fixes from mlaurent review
pinaraf marked 4 inline comments as done.Oct 31 2019, 7:41 PM

Fixes done

mlaurent added a comment.Nov 4 2019, 8:08 AM

you erase all other changes ?
I can see only feed.cpp

Revision Contents
Changeset List

PathPackages
Msrc/feed/feed.cpp (9 lines)

Diff 69134

View Options

src/feed/feed.cpp

Loading...