Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Get rid of "#if FEATURE_RANDOMIZED_STRING_HASHING" in shared partition.#10851

Merged
jkotas merged 1 commit intomasterfrom
unknown repository
Apr 10, 2017
Merged

Get rid of "#if FEATURE_RANDOMIZED_STRING_HASHING" in shared partition.#10851
jkotas merged 1 commit intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Apr 10, 2017

Since CoreCLR defines it (for now) and CoreRT doesn't, let's not have
"#if's" on this in shared code. It just creates confusion.

The only ones are in StringComparer.cs and fortunately, they are quite
expendable as...

  1. It's only purpose is serialize Hashtables and Dictionaries in
    a way that CLR 1.1 can deserialize them. We don't care about that in
    .NET Core.

  2. In CoreCLR as of today, this infrastructure is not changing the serialization
    blob one iota since all the GetEqualityComparerForSerialization() methods
    return "this."

So this removes the appearance of downgrading the comparers to non-randomized
versions for serialization purposes (and yes, we could go on and remove
all the other "let's do it for Everett and RTM" stuff in Hashtable.cs, but
I want to keep this PR on topic. Hashtable.cs is not going to be imported
into CoreRT so it's low priority for me.)

Since CoreCLR defines it (for now) and CoreRT doesn't, let's not have
"#if's" on this in shared code. It just creates confusion.

The only ones are in StringComparer.cs and fortunately, they are quite
expendable as...

1. It's only purpose is serialize Hashtables and Dictionaries in
   a way that CLR 1.1 can deserialize them. We don't care about that in
   .NET Core.

2. In CoreCLR as of today, this infrastructure is not changing the serialization
   blob one iota since all the GetEqualityComparerForSerialization() methods
   return "this."

So this removes the appearance of downgrading the comparers to non-randomized
versions for serialization purposes (and yes, we could go on and remove
all the other "let's do it for Everett and RTM" stuff in Hashtable.cs, but
I want to keep this PR on topic. Hashtable.cs is not going to be imported
into CoreRT so it's low priority for me.)
@ghost
Copy link
Author

ghost commented Apr 10, 2017

@dotnet-bot test Tizen armel Cross Debug Build

1 similar comment
@ghost
Copy link
Author

ghost commented Apr 10, 2017

@dotnet-bot test Tizen armel Cross Debug Build

@jkotas jkotas merged commit 1a32d7f into dotnet:master Apr 10, 2017
@ghost ghost deleted the delrand branch April 11, 2017 14:26
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants