Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

PYTHON-1309 cqlengine: Remove deepcopy on UserType deserialization #1192

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

Open
k0machi wants to merge 1 commit into datastax:master
base: master
Choose a base branch
Loading
from k0machi:cqlengine-remove-deepcopy-in-usertype-datastax

Conversation

Copy link

@k0machi k0machi commented Dec 4, 2023
edited by atlassian bot
Loading

This change makes it so newly instanced UserType during deserialization isn't immediately copied by deepcopy, which could cause huge slowdown if that UserType contains a lot of data or nested UserTypes, in which case the deepcopy calls would cascade as each to_python call would eventually clone parts of source object. As there isn't a lot of information on why this deepcopy is here in the first place this change could potentially break something. Running integration tests against this commit does not produce regressions, so this call looks safe to remove, but I'm leaving this warning here for the future reference.

Issue: scylladb#152
Issue (upstream): PYTHON-1309

Here are flamegraphs for an application that uses cqlengine and encounters this issue:

The request time goes from 10 seconds in this case down to 750ms, for reference the table is configured with a list of UserTypes that have a list of another UserType inside them.

This change makes it so newly instanced UserType during deserialization
isn't immediately copied by deepcopy, which could cause huge slowdown if
that UserType contains a lot of data or nested UserTypes, in which case the
deepcopy calls would cascade as each to_python call would eventually clone
parts of source object. As there isn't a lot of information on why this
deepcopy is here in the first place this change could potentially break
something. Running integration tests against this commit does not produce
regressions, so this call looks safe to remove, but I'm leaving this
warning here for the future reference.
Issue: scylladb#152 
Copy link

fruch commented Dec 13, 2023

@absurdfarce you think someone would be able to take a look at this one ? (there was no response on the related issue: https://datastax-oss.atlassian.net/browse/PYTHON-1309)

@k0machi k0machi changed the title (削除) cqlengine: Remove deepcopy on UserType deserialization (削除ここまで) (追記) PYTHON-1309 cqlengine: Remove deepcopy on UserType deserialization (追記ここまで) Dec 18, 2023
Copy link
Collaborator

Thanks for the PR @k0machi! Apologies, this somehow completely slipped past me when it first came in.

Have you signed the Contributor License Agreement for contributions to DataStax open source projects? If not you can find it at https://cla.datastax.com/. Thanks!

Copy link

fruch commented Jun 19, 2025

Thanks for the PR @k0machi! Apologies, this somehow completely slipped past me when it first came in.

Have you signed the Contributor License Agreement for contributions to DataStax open source projects? If not you can find it at https://cla.datastax.com/. Thanks!

@k0machi from my team in scylladb, a cla still needed ?

Also we have this change for quite some time in our fork, no with issues (so far...)

Copy link
Collaborator

Ah, sorry, I didn't realize that @fruch ... if that's the case then no, we're good.

Copy link

fruch commented Jul 4, 2025

Ah, sorry, I didn't realize that @fruch ... if that's the case then no, we're good.

Next time I'll tell him to be more vocal about where he's from 😜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /