Skip to content

[WIP] feat: Improve performances #1595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

[WIP] feat: Improve performances #1595

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
617acef
feat: Do not hydrate doc if the relationship does not exist
paultranvan Dec 17, 2024
5fb23bb
fix: Correctly fetch documents relationships
paultranvan Dec 17, 2024
df2b76d
feat: Improve store evaluation
paultranvan Dec 17, 2024
a19c594
feat: Improve persistVirtualDocuments performances
Ldoppea Feb 27, 2025
89645bb
test: Add more unit tests for jsonapi
Ldoppea Feb 28, 2025
83c3d92
feat: Edit docs instead of creating a new ones in fromPouchResult
Ldoppea Feb 28, 2025
19e9691
feat: TEMP use fast merge
Ldoppea Feb 28, 2025
5807df1
feat: TEMP ADD PERF LOGS
Ldoppea Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion packages/cozy-client/src/CozyClient.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,18 @@ client.query(Q('io.cozy.bills'))`)
if (!Array.isArray(data)) {
await this.persistVirtualDocument(data, enforce)
} else {
for (const document of data) {
const documentsToPersist = data.filter(document => {
if (!document || document.cozyLocalOnly) {
return false
}

if ((!document.meta?.rev && !document._rev) || enforce) {
return true
}

return false
})
for (const document of documentsToPersist) {
Copy link
Contributor

@Crash-- Crash-- Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really better? OK you don't call a method if not needed, but now you loop twice

Copy link
Contributor

@paultranvan paultranvan Mar 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually significantly better. After a big query retrieving 25K docs, the persistVirtualDocuments takes:

  • Before: 609ms
  • After: 20ms

I wonder if this is because of the event loop overwhelmed by too many calls

Copy link
Contributor

@Crash-- Crash-- Mar 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh shit, I didn't saw that the persistVirtualDocument was async... This is why.

nit: You can now remove the check within the persistVirtualDocument method since should only call it with the docs to persist ^^.

await this.persistVirtualDocument(document, enforce)
}
}
Expand Down