Skip to content

Conversation

@grigi
Copy link
Member

@grigi grigi commented Jul 19, 2019

So, this PR started off as me trying to add a post-process hook to model creation, so that we can try and have a single place to do generate pre-compute lookups.

Then I saw all the memoised properties, and thought that pre-generating this would be a good exercised of if the post-process hooks get called correctly.

It actually turns out we have 4 places where these updates can happen:

  1. Model parsed
  2. Model initialised (for pk) → This one feels non-obvious to me.
  3. After relationships created
  4. DB associated (still done in Tortoise.init(), this PR does not change this)

Also split constructor into from-python and from-db so we can unblock #55

This lead to significant perf improvements in the code, especially for large selects.

ALSO:

Manipulating PyPika objects directly, 5-30% speedup for small fetches.
Not too sure about this, as it is touching some private variables. Its safe as we manually copy the PyPika object before manipulating it, but now we ALWAYS copy once, instead of for every transform.

@grigi grigi requested a review from abondar July 19, 2019 12:24
@grigi
Copy link
Member Author

grigi commented Jul 19, 2019

@abondar Please review

Copy link
Member

@abondar abondar left a comment

Choose a reason for hiding this comment

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

Seems legit to me

value = value.id
else:
db_field = self.model._meta.fields_db_projection[key]
# self.query._updates.append((Field(db_field), ValueWrapper(value))
Copy link
Member

Choose a reason for hiding this comment

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

I think that line should be deleted :)

@grigi grigi merged commit 5f6bd8c into master Jul 21, 2019
@grigi grigi deleted the metadata_perf branch August 24, 2019 05:15
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.

3 participants