Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix perf regression in PR #3530 #3544

Merged
merged 4 commits into from
Jul 17, 2018
Merged

Conversation

erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Jul 17, 2018

The get_entities_changed function was changed to return all changed
entities since the given stream position, rather than only those changed
from a given list of entities. This resulted in the function incorrectly
returning large numbers of entities that, for example, caused large
increases in database usage.

(fixes a regression in #3530)

The get_entities_changed function was changed to return all changed
entities since the given stream position, rather than only those changed
from a given list of entities. This resulted in the function incorrectly
returning large numbers of entities that, for example, caused large
increases in database usage.
@erikjohnston erikjohnston requested a review from a team July 17, 2018 09:49
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

can haz UT pls?

self._cache[k] for k in self._cache.islice(
start=self._cache.bisect_right(stream_pos),
)
}

result = {
Copy link
Member

Choose a reason for hiding this comment

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

it'll be quicker to do result = changed_entities.intersection(entities) here, since intersection is an optimised bit of C (which does do the right thing when given a non-set argument)

Copy link
Member Author

Choose a reason for hiding this comment

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

ooh, cool

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm

@erikjohnston erikjohnston merged commit c7320a5 into develop Jul 17, 2018
@erikjohnston erikjohnston deleted the erikj/fixup_stream_cache branch September 20, 2018 13:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants