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.

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

Prevent single file storage from performing unnecessary N^2 loop #1051

Merged
merged 1 commit into from
Mar 10, 2023

Conversation

eguiraud
Copy link
Contributor

For single file storage we wrap the logic of get_multi with the at_once context manager so that self.list() (which is called by self.get()) actually caches the items rather than re-computing them at every call.

This should largely mitigate the performance issue describe at #818 . The issue discussion also contains more background about this patch.

For single file storage we wrap the logic of get_multi with the
at_once context manager so that `self.list()` (which is called by
`self.get()`) actually caches the items rather than re-computing
them at every call.

This should largely mitigate the performance issue describe at
pimutils#818 . The issue
discussion also contains more background about this patch.
Copy link
Member

@WhyNotHugo WhyNotHugo left a comment

Choose a reason for hiding this comment

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

Perfect, thanks!

@WhyNotHugo
Copy link
Member

CI is failing because the cache is never initialized during multi-get, even if it's not already.

It always needs to happen if self._items is None.

@WhyNotHugo
Copy link
Member

I wonder if the failure scenario is only possible in tests or also happens in some untested real-life scenario.

@eguiraud
Copy link
Contributor Author

I don't understand how this can happen:

vdirsyncer/storage/singlefile.py:138: in get_multi
    async with self.at_once():
/usr/lib/python3.10/contextlib.py:206: in __aexit__
    await anext(self.gen)
vdirsyncer/storage/singlefile.py:200: in at_once
    self._write()
vdirsyncer/storage/singlefile.py:186: in _write
    text = join_collection(item.raw for item, etag in self._items.values())
E   AttributeError: 'NoneType' object has no attribute 'values'

at_once calls self.list() first thing, and list() calls self._items = collections.OrderedDict(). So I would expect _items to be initialized to an empty dict by the time at_once calls _write?

@WhyNotHugo
Copy link
Member

See b1ef680

@WhyNotHugo WhyNotHugo merged commit 85ae339 into pimutils:main Mar 10, 2023
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.

None yet

2 participants