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

Remove dependency on System.Collections.Immutable #78

Closed
mikegoatly opened this issue Aug 17, 2023 · 2 comments
Closed

Remove dependency on System.Collections.Immutable #78

mikegoatly opened this issue Aug 17, 2023 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@mikegoatly
Copy link
Owner

mikegoatly commented Aug 17, 2023

ImTools looks like it provides almost drop-in replacements for classes like ImmutableDictionary that are used very heavily in LIFTI.

According to the benchmarks they will provide a 2x read speed improvement in our scenarios, and a 6x speed improvement for heavily used operations during indexing.

It would be worth experimenting to see if the perf benefits really do play out.

My only concern is that right now the core library only takes a dependency on a MS-supported library - would taking a dependency on another OSS library be off-putting for anyone?

(I've never officially committed to using only Microsoft-led dependencies, it's just a co-incidence that's where the implementation landed)

@mikegoatly mikegoatly added enhancement New feature or request help wanted Extra attention is needed labels Aug 17, 2023
@samba2
Copy link

samba2 commented Aug 20, 2023

In our team we don't mind bringing in libs as long as they are 'dry-aged' (around for a long time) and have many contributors. Particularly the last one I find important. Otherwise you can get into trouble when the maintainer looses interest.
So I'd tend to not add the lib as it seems to have only one contributor.

@mikegoatly mikegoatly added this to the v6 milestone Dec 22, 2023
@mikegoatly
Copy link
Owner Author

mikegoatly commented Dec 22, 2023

I've decided to the System.Collections.Immutable dependency completely. I can keep the immutable semantics without leaking ImmutableDictionarys everywhere, initial investigations show that perf and memory is unaffected, and in some cases (index deserialization) perf is actually improved.

@mikegoatly mikegoatly changed the title Performance: Use ImTools instead of System.Collection.Immutable Remove dependency on System.Collections.Immutable Dec 22, 2023
mikegoatly added a commit that referenced this issue Dec 22, 2023
Removed dependency on System.Collections.Immutable #78
mikegoatly added a commit that referenced this issue Jan 6, 2024
Removed dependency on System.Collections.Immutable #78
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants