-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Adds perf tests for Map.merge
.
#1407
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
Conversation
@leebyron please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks for adding to these!
perf/Map.js
Outdated
} | ||
|
||
const m1 = Immutable.Map(obj); | ||
const m2 = Immutable.Map(obj); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two are the same - there's a special path for no-op merges, so you might want to add a separate perf test where the second map contains different values
perf/Map.js
Outdated
@@ -139,4 +139,39 @@ describe('Map', function () { | |||
|
|||
}); | |||
|
|||
describe('merge an object', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'merge a map'
?
…nces (and content).
@leebyron Done. Thanks! Are you interested in me getting the perf tests running in Travis? |
@leebyron Resolved (style) conflicts. |
I think the perf tests might be too slow to run in Travis, also they're pretty sensitive the the operating environment so I think they may also be too noisy |
It wasn't actually running mergeDeep() and as a perf test, it's a thin composition atop merge() anyhow.
Map.merge
and Map.mergeDeep
.Map.merge
.
Thanks! I added a few additional changes. I followed up with the change I mentioned before about ensuring the second map contains different values. I also removed |
* facebook/master: (305 commits) Reduce repetition in COC link `CONTRIBUTING.md` Add CODE_OF_CONDUCT.md fix typo Add link to Code of Conduct in CONTRIBUTING Improved Flow support for Record subclasses (immutable-js#1414) Improve asImmutable() docs (immutable-js#1415) Only include version in require statement in Runkit instances. (immutable-js#1411) Add example for OrderedSet subtract. (immutable-js#1410) Fix code samples in Immutable.d.ts docs (immutable-js#1408) Adds perf tests for `Map.merge`. (immutable-js#1407) Move gulpfile into resources/ Prettier: perf tests Prettier: include resources Prettify: Use es5 trailing commas Merge immutable-js#1403 Document regression test steps. (immutable-js#1405) Demonstrates correct handling of Symbol keys in Map.mergeDeep when the Maps are nested, and initialized with Symbol KV tuples instead of plain JS object literals. (immutable-js#1404) Fix Map#concat (immutable-js#1402) test(ts-definitions): test Immutable.d.ts validity using tsc (immutable-js#1401) 4.0.0-rc.9 ...
* facebook/master: (305 commits) Reduce repetition in COC link `CONTRIBUTING.md` Add CODE_OF_CONDUCT.md fix typo Add link to Code of Conduct in CONTRIBUTING Improved Flow support for Record subclasses (immutable-js#1414) Improve asImmutable() docs (immutable-js#1415) Only include version in require statement in Runkit instances. (immutable-js#1411) Add example for OrderedSet subtract. (immutable-js#1410) Fix code samples in Immutable.d.ts docs (immutable-js#1408) Adds perf tests for `Map.merge`. (immutable-js#1407) Move gulpfile into resources/ Prettier: perf tests Prettier: include resources Prettify: Use es5 trailing commas Merge immutable-js#1403 Document regression test steps. (immutable-js#1405) Demonstrates correct handling of Symbol keys in Map.mergeDeep when the Maps are nested, and initialized with Symbol KV tuples instead of plain JS object literals. (immutable-js#1404) Fix Map#concat (immutable-js#1402) test(ts-definitions): test Immutable.d.ts validity using tsc (immutable-js#1401) 4.0.0-rc.9 ...
Output sample:
Full output: map-merge-perf.txt