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

Copyable classes with bindable properties #4251

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

balex89
Copy link
Contributor

@balex89 balex89 commented Jan 22, 2025

This PR follows #3995 issue discussion.

I went with Option 3 for a list of reasons:

  1. It uncompromisingly covers all aspects of the issue, including tracking of bindable attributes;
  2. It doesn't really affect performance much, if done right (see below);
  3. It isn't in fact as ridiculously complicated in terms of implementation as it might seem, once you see that it only adds a wrapper for function that reproduces an object instance.

Impact on performance

Since we modify BindableProperty setter method, i ran a simple test to measure impact on code performance when object is a) initialized and b) updated:

class TestClass:

    x = BindableProperty()

    def __init__(self):
        self.x = 1

    def update(self):
        self.x = 2

init_time = timeit.timeit(lambda: TestClass(), number=1_000_000)
a = TestClass()
update_time = timeit.timeit(lambda: a.update(), number=1_000_000)
print(f'creation: {init_time:.6f} s, update: {update_time:.6f} s')

I ran this test several times for the current and updated BindableProperty implementations and got following average figures:

  1. For initialization it adds up to 5% performance time on average;
  2. For update it adds ~1% (or less) time on average.

Since a user, aiming to create millions of instances, each meant for binding, will probably face more sever issues for other reasons, the real-life overhead seems to be insignificant.

@balex89 balex89 force-pushed the copyable-bindability branch from bc67683 to f9d1d4d Compare January 23, 2025 10:02
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.

1 participant