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

RavenDB-13921 upgraded Jint #9614

Merged
merged 8 commits into from
Aug 21, 2019
Merged

RavenDB-13921 upgraded Jint #9614

merged 8 commits into from
Aug 21, 2019

Conversation

ppekrol
Copy link
Member

@ppekrol ppekrol commented Aug 21, 2019

No description provided.

public Dictionary<string, BlittableObjectProperty> OwnValues =
new Dictionary<string, BlittableObjectProperty>();
public HashSet<Key> Deletes;
public Dictionary<Key, BlittableObjectProperty> OwnValues = new Dictionary<Key, BlittableObjectProperty>();
Copy link
Contributor

Choose a reason for hiding this comment

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

As you anyway have custom build of Jint, there's also possibility to make StringDictionarySlim public and use it. It's basically the tuned version of Dictionary (some history here).

Copy link
Member Author

Choose a reason for hiding this comment

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

Or maybe it is better to include 'Microsoft.Experimental.Collections' and use DictionarySlim<TK, TV> instead of Jint's string version. What are benefits of StringDictionarySlim<T> vs DictionarySlim<string, T> or DictionarySlim<Jint.Key, T> ?

Last one seems like the most reasonable option, because we would have an opportunity to take leverage from Key. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably the most convenient path is to use the MS Experimental. Jint version just skips GetHashCode etc and accesses the struct field directly.

@ppekrol ppekrol merged commit df47ed6 into ravendb:v5.0 Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants