Skip to content

perf(criteria): filter-out older views before sorting articles read in session#1172

Draft
miguelpeixe wants to merge 7 commits into
masterfrom
perf/improve-sorting-articles-read
Draft

perf(criteria): filter-out older views before sorting articles read in session#1172
miguelpeixe wants to merge 7 commits into
masterfrom
perf/improve-sorting-articles-read

Conversation

@miguelpeixe
Copy link
Copy Markdown
Member

All Submissions:

Changes proposed in this Pull Request:

Set a 6 hours threshold to filter out views before sorting and calculating the articles read in session. Even though a session can stretch for several hours if the reader keeps reading articles, it's not likely to get longer than 6 hours.

Currently, the sorting will run over every article read in the store history, which is not necessary and can have some performance impact.

How to test the changes in this Pull Request:

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

miguelpeixe and others added 5 commits July 13, 2023 15:14
* chore: remove Lightweight API and client ID handling

* chore: remove data pruning cron job and clear future instances

* fix: script dependencies

* fix: remove unused method refs

* test: remove tests using deprecated features

* chore: remove more unneeded files

* test: remove other failing tests
Co-authored-by: Leo Germani <leogermani@automattic.com>
Base automatically changed from epic/rearchitecture to master August 17, 2023 20:40
@dkoo
Copy link
Copy Markdown
Contributor

dkoo commented Aug 21, 2023

@miguelpeixe can we close this one?

@dkoo dkoo closed this Aug 21, 2023
@dkoo dkoo reopened this Aug 21, 2023
@miguelpeixe
Copy link
Copy Markdown
Member Author

Thanks for reminding me @dkoo! This is actually still a relevant, but not urgent, performance tweak to the "articles read" matching function. We don't need this for the epic release, I'll make this PR available for review later.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants