Skip to content

PYTHON-4779 Use slots for Selection and TopologyDescription #2414

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

Closed
wants to merge 3 commits into from

Conversation

blink1073
Copy link
Member

@blink1073 blink1073 requested a review from ShaneHarvey July 1, 2025 20:47
@blink1073 blink1073 requested a review from a team as a code owner July 1, 2025 20:47
Copy link
Member

@ShaneHarvey ShaneHarvey left a comment

Choose a reason for hiding this comment

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

Could you add something like this to the changelog?

- Classes :class:`~bson.int64.Int64`, :class:`~bson.min_key.MinKey`,
  :class:`~bson.max_key.MaxKey`, :class:`~bson.timestamp.Timestamp`,
  :class:`~bson.regex.Regex`, and :class:`~bson.dbref.DBRef` all implement
  ``__slots__`` now. This means that their attributes are fixed, and new
  attributes cannot be added to them at runtime.

Also is there any measurable benefit at all to this change? Like what if we have 1 thread just doing client._select_server? Or 100 threads?

@blink1073
Copy link
Member Author

I see no significant differences with this script between this PR and master:

from pymongo import MongoClient
from pymongo.read_preferences import ReadPreference
from concurrent.futures import ThreadPoolExecutor, wait

client = MongoClient()
session = client.start_session()
read_preference = ReadPreference.PRIMARY


def test():
    for i in range(100):
        client._select_server(read_preference, session, "testOperation")



def test2_target():
    client._select_server(read_preference, session, "testOperation")


def test2():
    futures = []
    with ThreadPoolExecutor(max_workers=100) as pool:
        for i in range(100):
            futures.append(pool.submit(test2_target))
        wait(futures)
    

if __name__ == '__main__':
    import timeit
    print(timeit.timeit("test()", setup="from __main__ import test, client, read_preference, session", number=1000))
    print(timeit.timeit("test2()", setup="from __main__ import test2, client, read_preference, session", number=1000))

@blink1073
Copy link
Member Author

Given that there is no measurable improvement, and Selection isn't even represented in our API docs, I think we should close this PR and ticket.

@blink1073 blink1073 requested a review from ShaneHarvey July 2, 2025 01:37
@ShaneHarvey
Copy link
Member

Closing sounds good to me. We should only do this if we can show it leads to a measurable improvement.

@blink1073 blink1073 closed this Jul 2, 2025
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.

2 participants