ytatichno commented on PR #652: URL: https://github.com/apache/commons-collections/pull/652#issuecomment-4724907048
Hello @garydgregory It's kind of hard to mock JRE classes for application level testing without unpleasant experience, so I took the core logic from invertMap and created a small isolated test that proves the improvement. [The test](https://github.com/apache/commons-collections/pull/652/changes/98591c3e86f0dc83f68e1c9cf2db719f550400c2) uses an input map with 14 entries. It builds two output maps: one using the original capacity formula and one using the proposed formula and adds the same entries in the same order. After each put, it checks via reflection whether the internal table array has changed identity. The first change on each map (from null to a freshly allocated array) is the lazy initialization; any subsequent change is a genuine resize. I see that link for old formula changed twice: one time for initialization from null to not null link and one extra time that we perhaps don't want to have. And the link for new formula changed only once for initialization from null to freshly allocated array link. Note that on Java 9+ it requires `--add-opens java.base/java.util=ALL-UNNAMED` to access the table field from internals of java.util.HashMap via Reflection. That's why [this test](https://github.com/apache/commons-collections/pull/652/changes/98591c3e86f0dc83f68e1c9cf2db719f550400c2) shouldn't be included anywhere else. Regarding the 0.75d in the patch: would you prefer it to be extracted as a named constant, or is a short explanatory comment sufficient? And as You remember, this extra resize triggered for 50% of sizes of input map and won't be reproduced on 12 elements test i.e. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
