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

Performance issue with FLLarge FLLargeIdentityDictionary and FLLargeI… #294

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

Conversation

ELePors
Copy link

@ELePors ELePors commented Sep 6, 2024

Modify references to FLLarge... to use standard Pharo objects.
Performance is increased in my tests, but i don't know the full impactof this modification ... ?

Cheers
Eric.

@tinchodias
Copy link
Collaborator

The change may be correct for current images.
The purpose of FLLargeIdentityDictionary was being polymorphic and faster than IdentityDictionary when a large number of key/values are stored... and it wasn't slower with small number of key/values too AFAIR.
We benchmarked on 32-bits Pharo, maybe 1.1 ?!?!
Maybe @marianopeck remembers more details.

Copy link

stale bot commented Nov 10, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will remain open but will probably not come into focus. If you still think this should receive some attention, leave a comment. Thank you for your contributions.

@stale stale bot added the stale label Nov 10, 2024
Copy link
Owner

@theseion theseion left a comment

Choose a reason for hiding this comment

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

Please remove all custom hashed collections completely.

@stale stale bot removed the stale label Nov 10, 2024
@theseion
Copy link
Owner

@tinchodias as far as I can tell, drawing on the benchmarks that Guille did, the custom collections no longer offer any advantages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants