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

Use collision-resistent maps for formFieldMaps #2274

Merged
merged 2 commits into from Nov 20, 2018

Conversation

Projects
None yet
5 participants
@raboof
Copy link
Member

raboof commented Nov 8, 2018

No description provided.

@akka-ci

This comment has been minimized.

Copy link
Collaborator

akka-ci commented Nov 8, 2018

Test FAILed.

@raboof raboof changed the title Use collision-resistent hashes for formFieldMaps Use collision-resistent maps for formFieldMaps Nov 8, 2018

@akka-ci

This comment has been minimized.

Copy link
Collaborator

akka-ci commented Nov 8, 2018

Test PASSed.

@jlprat

jlprat approved these changes Nov 19, 2018

Copy link
Member

jlprat left a comment

LGTM

Pretty impressive the difference in numbers without this fix. On my machine using plain maps the numbers are:

[info] - should not show bad performance characteristics when field names' hashCodes collide *** FAILED ***
[info]   57 was not less than 2 (FormFieldDirectivesSpec.scala:212)
[info] - should not show bad performance characteristics when field names' hashCodes collide *** FAILED ***
[info]   97 was not less than 2 (FormFieldDirectivesSpec.scala:262)

@raboof raboof added this to the 10.1.6 milestone Nov 20, 2018

@jrudolph
Copy link
Member

jrudolph left a comment

LGTM

@@ -104,11 +104,13 @@ object FormFieldDirectives extends FormFieldDirectives {

_formFieldSeq.map {
case seq
append(Map.empty, seq)
append(immutable.TreeMap.empty, seq)

This comment has been minimized.

@jrudolph

jrudolph Nov 20, 2018

Member

Yep, I guess using TreeMap is fine for form fields. Having hundreds of those wouldn't be best practice anyway.

@jrudolph jrudolph merged commit 33871be into master Nov 20, 2018

4 checks passed

Jenkins PR Validation Test PASSed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
typesafe-cla-validator All users have signed the CLA
Details

@jrudolph jrudolph deleted the collisionResistentMaps branch Nov 20, 2018

jrudolph added a commit to jrudolph/akka-http that referenced this pull request Dec 20, 2018

jrudolph added a commit to jrudolph/akka-http that referenced this pull request Dec 20, 2018

raboof added a commit that referenced this pull request Dec 21, 2018

[10.0 backport] Collision-resistant hashMap fixes (#2343)
* [backport] Use collision-resistent maps for formFieldMaps (#2274)

(cherry picked from commit 33871be)

* [backport] collision-resistent maps in header parsing (#2276)

(cherry picked from commit 6b224f1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment