Skip to content
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

Jump to bottom

Backup dedup performance #1100

Merged
merged 5 commits into from
Jan 4, 2024
Merged

Backup dedup performance #1100

merged 5 commits into from
Jan 4, 2024

Conversation

zatteo
Copy link
Contributor

@zatteo zatteo commented Jan 4, 2024 •
edited
Loading

  • improve backup dedup performance (10x)
  • improve backup dedup readability

@zatteo zatteo force-pushed the feat/backup-dedup-performance-bis branch from 3f02f88 to 9b19981 Compare January 4, 2024 10:00
@zatteo zatteo marked this pull request as ready for review January 4, 2024 10:05
@zatteo zatteo changed the title Feat/backup dedup performance bis Backup dedup performance Jan 4, 2024
@zatteo zatteo marked this pull request as draft January 4, 2024 10:43
@zatteo
feat(backup): Return immediatly if names are different in dedup compa…
2c1ecba 
…rison function

It will improve speed A LOT by :
- avoiding instanciating dates for nothing
- avoiding getting flag for nothing
@zatteo
feat(backup): Remove useless call to increase performance
b393251 
Since we compare date with full year, month, day, minutes and secondes, we do not need anymore to set milliseconds to 0. So we can remove this useless call.
@zatteo zatteo force-pushed the feat/backup-dedup-performance-bis branch from 18326d5 to b393251 Compare January 4, 2024 10:45
@zatteo zatteo marked this pull request as ready for review January 4, 2024 10:50
Ldoppea
Ldoppea approved these changes Jan 4, 2024
View reviewed changes
Copy link
Member

@Ldoppea Ldoppea left a comment

Choose a reason for hiding this comment

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

You mention the fact that you want to avoid unnecessary new Date() calls

Does this mean that date comparizon is heavy enough to be a thing we want to optimize?

If yes, is there any chance that using date-fns would be more effective at comparing dates?

@zatteo
Copy link
Contributor Author

zatteo commented Jan 4, 2024

You mention the fact that you want to avoid unnecessary new Date() calls

Does this mean that date comparizon is heavy enough to be a thing we want to optimize?

If yes, is there any chance that using date-fns would be more effective at comparing dates?

Interesting I will keep that in mind especially if it can fasten the comparison in itself.

For the record, I tried to instanciate every date object before the loop to avoid new Date calls, but after the "name comparison first" fix, it gave unsignificant results but complicated the code.

@zatteo zatteo merged commit 489b387 into master Jan 4, 2024
1 check passed
@zatteo zatteo deleted the feat/backup-dedup-performance-bis branch January 4, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ldoppea Ldoppea Ldoppea approved these changes

@acezard acezard Awaiting requested review from acezard acezard is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zatteo @Ldoppea