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.

Sign up for GitHub

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

Jump to bottom

new LinkedHashMap/LinkedHashSet implementation #10221

Merged
merged 3 commits into from Dec 2, 2022
Merged

new LinkedHashMap/LinkedHashSet implementation #10221

merged 3 commits into from Dec 2, 2022

Conversation

liang3zy22
Copy link
Contributor

@liang3zy22 liang3zy22 commented Nov 17, 2022 •
edited by SethTisue

This PR is to fix scala/bug#11369. The old PR is closed, so create new one. The benchmark data will be added if all tests pass.

@scala-jenkins scala-jenkins added this to the 2.13.11 milestone Nov 17, 2022
@liang3zy22 liang3zy22 marked this pull request as draft November 17, 2022 04:05
@liang3zy22
Copy link
Contributor Author

liang3zy22 commented Nov 22, 2022 •
edited

All tests passed. So I have run some benchmark testing to get some performance data. I copied mutable/HashMapBenchmark2.scala and made the linkedhashmap/linkedhashset version. Please give comment to improve the benchmark program if needed.

The old linkedhashmap data:
Benchmark                           (size)  (stringsOnly)  Mode  Cnt         Score               Error   Units
LinkedHashMapBenchmark2.lhmBuild               10           true  avgt   40       158.949 ±       1.056  ns/op
LinkedHashMapBenchmark2.lhmBuild              100           true  avgt   40      2990.460 ±      84.548  ns/op
LinkedHashMapBenchmark2.lhmBuild             1000           true  avgt   40     35159.047 ±     354.869  ns/op
LinkedHashMapBenchmark2.lhmBuild            10000           true  avgt   40    553150.859 ±    2310.097  ns/op
LinkedHashMapBenchmark2.lhmFillColliding       10           true  avgt   40       236.445 ±       1.12   ns/op
LinkedHashMapBenchmark2.lhmFillColliding      100           true  avgt   40      7262.521 ±      42.028  ns/op
LinkedHashMapBenchmark2.lhmFillColliding     1000           true  avgt   40    281634.118 ±    844.832   ns/op
LinkedHashMapBenchmark2.lhmFillColliding    10000           true  avgt   40  22332434.136 ±  1621274.211 ns/op
LinkedHashMapBenchmark2.lhmFillRegular         10           true  avgt   40       124.998 ±       1.464  ns/op
LinkedHashMapBenchmark2.lhmFillRegular        100           true  avgt   40      2622.406 ±      31.329  ns/op
LinkedHashMapBenchmark2.lhmFillRegular       1000           true  avgt   40     32016.401 ±     133.902  ns/op
LinkedHashMapBenchmark2.lhmFillRegular      10000           true  avgt   40    508360.072 ±    1930.748  ns/op
LinkedHashMapBenchmark2.lhmGetExisting         10           true  avgt   40        84.727 ±       0.391  ns/op
LinkedHashMapBenchmark2.lhmGetExisting        100           true  avgt   40       883.492 ±      1.615   ns/op
LinkedHashMapBenchmark2.lhmGetExisting       1000           true  avgt   40     10246.926 ±     198.531  ns/op
LinkedHashMapBenchmark2.lhmGetExisting      10000           true  avgt   40    138553.664 ±     2615.972 ns/op
LinkedHashMapBenchmark2.lhmGetNone             10           true  avgt   40       814.774 ±       3.964  ns/op
LinkedHashMapBenchmark2.lhmGetNone            100           true  avgt   40       765.882 ±       5.509  ns/op
LinkedHashMapBenchmark2.lhmGetNone           1000           true  avgt   40      9492.448 ±      38.564  ns/op
LinkedHashMapBenchmark2.lhmGetNone          10000           true  avgt   40    177260.698 ±    490.294   ns/op
LinkedHashMapBenchmark2.lhmIterateEntries      10           true  avgt   40        70.186 ±       0.245  ns/op
LinkedHashMapBenchmark2.lhmIterateEntries     100           true  avgt   40      700.647 ±        2.379  ns/op
LinkedHashMapBenchmark2.lhmIterateEntries    1000           true  avgt   40     7091.794 ±      20.849   ns/op
LinkedHashMapBenchmark2.lhmIterateEntries   10000           true  avgt   40    71360.056 ±    214.509    ns/op
LinkedHashMapBenchmark2.lhmIterateKeys         10           true  avgt   40        37.512 ±       0.026  ns/op
LinkedHashMapBenchmark2.lhmIterateKeys        100           true  avgt   40       373.153 ±       0.342  ns/op
LinkedHashMapBenchmark2.lhmIterateKeys       1000           true  avgt   40      3729.026 ±      8.098   ns/op
LinkedHashMapBenchmark2.lhmIterateKeys      10000           true  avgt   40     37082.995 ±    184.722   ns/op

The new linkedhashmap data:
Benchmark                           (size)  (stringsOnly)  Mode  Cnt         Score               Error   Units
LinkedHashMapBenchmark2.lhmBuild               10           true  avgt   40       124.853 ±       1.595  ns/op
LinkedHashMapBenchmark2.lhmBuild              100           true  avgt   40      2350.852 ±       6.331  ns/op
LinkedHashMapBenchmark2.lhmBuild             1000           true  avgt   40     21075.987 ±       152.6  ns/op
LinkedHashMapBenchmark2.lhmBuild            10000           true  avgt   40    249774.548 ±     577.924  ns/op
LinkedHashMapBenchmark2.lhmFillColliding       10           true  avgt   40       215.237 ±        2.46  ns/op
LinkedHashMapBenchmark2.lhmFillColliding      100           true  avgt   40      6923.775 ±      108.951 ns/op
LinkedHashMapBenchmark2.lhmFillColliding     1000           true  avgt   40    397472.022 ±   20754.361  ns/op
LinkedHashMapBenchmark2.lhmFillColliding    10000           true  avgt   40  20627891.831 ±   300042.792 ns/op
LinkedHashMapBenchmark2.lhmFillRegular         10           true  avgt   40       97.391 ±        1.039  ns/op
LinkedHashMapBenchmark2.lhmFillRegular        100           true  avgt   40      2073.675 ±      13.661  ns/op
LinkedHashMapBenchmark2.lhmFillRegular       1000           true  avgt   40     18328.545 ±      57.867  ns/op
LinkedHashMapBenchmark2.lhmFillRegular      10000           true  avgt   40    212200.734 ±     2901.91  ns/op
LinkedHashMapBenchmark2.lhmGetExisting         10           true  avgt   40        51.841 ±       0.296  ns/op
LinkedHashMapBenchmark2.lhmGetExisting        100           true  avgt   40       574.248 ±      7.936   ns/op
LinkedHashMapBenchmark2.lhmGetExisting       1000           true  avgt   40     6196.052 ±       36.132  ns/op
LinkedHashMapBenchmark2.lhmGetExisting      10000           true  avgt   40    65884.552 ±      751.892  ns/op
LinkedHashMapBenchmark2.lhmGetNone             10           true  avgt   40       569.892 ±        2.37  ns/op
LinkedHashMapBenchmark2.lhmGetNone            100           true  avgt   40       571.078 ±       4.027  ns/op
LinkedHashMapBenchmark2.lhmGetNone           1000           true  avgt   40      5816.213 ±       3.813  ns/op
LinkedHashMapBenchmark2.lhmGetNone          10000           true  avgt   40    71065.408 ±      65.914   ns/op
LinkedHashMapBenchmark2.lhmIterateEntries      10           true  avgt   40        70.197 ±       0.335  ns/op
LinkedHashMapBenchmark2.lhmIterateEntries     100           true  avgt   40      700.298 ±        2.379  ns/op
LinkedHashMapBenchmark2.lhmIterateEntries    1000           true  avgt   40     7035.085 ±       25.08   ns/op
LinkedHashMapBenchmark2.lhmIterateEntries   10000           true  avgt   40    71281.895 ±    129.859    ns/op
LinkedHashMapBenchmark2.lhmIterateKeys         10           true  avgt   40        37.394 ±       0.108  ns/op
LinkedHashMapBenchmark2.lhmIterateKeys        100           true  avgt   40       372.750 ±       1.251  ns/op
LinkedHashMapBenchmark2.lhmIterateKeys       1000           true  avgt   40      3691.366 ±     16.513   ns/op
LinkedHashMapBenchmark2.lhmIterateKeys      10000           true  avgt   40     37301.999 ±     19.638   ns/op

The java linkedhashmap data:
Benchmark                           (size)  (stringsOnly)  Mode  Cnt         Score               Error   Units
LinkedHashMapBenchmark2.javalhmBuild               10       true  avgt   40       175.725 ±       1.631  ns/op
LinkedHashMapBenchmark2.javalhmBuild              100       true  avgt   40      1646.287 ±      12.801  ns/op
LinkedHashMapBenchmark2.javalhmBuild             1000       true  avgt   40     16930.162 ±      61.396  ns/op
LinkedHashMapBenchmark2.javalhmBuild            10000       true  avgt   40     172560.806 ±    7749.79  ns/op
LinkedHashMapBenchmark2.javalhmFillColliding        10      true  avgt   40      274.291 ±        5.264  ns/op
LinkedHashMapBenchmark2.javalhmFillColliding      100       true  avgt   40      16538.535 ±     189.583 ns/op
LinkedHashMapBenchmark2.javalhmFillColliding     1000       true  avgt   40    662302.095 ±    5952.821  ns/op
LinkedHashMapBenchmark2.javalhmFillColliding    10000       true  avgt   40  35332959.830 ±   127230.326 ns/op
LinkedHashMapBenchmark2.javalhmFillRegular         10       true  avgt   40       170.175 ±       1.065  ns/op
LinkedHashMapBenchmark2.javalhmFillRegular        100       true  avgt   40      2396.858 ±      14.303  ns/op
LinkedHashMapBenchmark2.javalhmFillRegular       1000       true  avgt   40     23274.750 ±    318.692   ns/op
LinkedHashMapBenchmark2.javalhmFillRegular      10000       true  avgt   40    237210.010 ±     930.64   ns/op
LinkedHashMapBenchmark2.javalhmGetExisting         10       true  avgt   40        64.542 ±        1.36  ns/op
LinkedHashMapBenchmark2.javalhmGetExisting        100       true  avgt   40       589.552 ±      0.761   ns/op
LinkedHashMapBenchmark2.javalhmGetExisting       1000       true  avgt   40     6337.434 ±       18.198  ns/op
LinkedHashMapBenchmark2.javalhmGetExisting      10000       true  avgt   40    76856.636 ±      291.634  ns/op
LinkedHashMapBenchmark2.javalhmGetNone             10       true  avgt   40       633.984 ±       4.441  ns/op
LinkedHashMapBenchmark2.javalhmGetNone            100       true  avgt   40       630.037 ±       2.505  ns/op
LinkedHashMapBenchmark2.javalhmGetNone           1000       true  avgt   40      6868.226 ±      30.128  ns/op
LinkedHashMapBenchmark2.javalhmGetNone          10000       true  avgt   40    93617.622 ±     718.897   ns/op
LinkedHashMapBenchmark2.javalhmIterateEntries      10       true  avgt   40        53.579 ±       0.323  ns/op
LinkedHashMapBenchmark2.javalhmIterateEntries     100       true  avgt   40      526.805 ±        0.624  ns/op
LinkedHashMapBenchmark2.javalhmIterateEntries    1000       true  avgt   40      4884.234 ±      9.823   ns/op
LinkedHashMapBenchmark2.javalhmIterateEntries   10000       true  avgt   40     53239.457 ±   304.493    ns/op
LinkedHashMapBenchmark2.javalhmIterateKeys         10       true  avgt   40        53.742 ±       0.199  ns/op
LinkedHashMapBenchmark2.javalhmIterateKeys        100       true  avgt   40       531.818 ±       3.267  ns/op
LinkedHashMapBenchmark2.javalhmIterateKeys       1000       true  avgt   40      4854.501 ±     30.117   ns/op
LinkedHashMapBenchmark2.javalhmIterateKeys       10000      true  avgt   40     53733.450 ±    316.574   ns/op

@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Nov 22, 2022
@SethTisue SethTisue requested a review from a team November 22, 2022 21:58
@SethTisue SethTisue added the library:collections PRs involving changes to the standard collection library label Nov 22, 2022
@liang3zy22
Copy link
Contributor Author

liang3zy22 commented Nov 23, 2022 •
edited

The linkedhashset benchmarking data is as below:

Old linkedhashset benchmark data:
Benchmark                                 (size)  (stringsOnly)  Mode  Cnt         Score        Error  Units
LinkedHashMapBenchmark2.lhsBuild              10           true  avgt   40        142.391 ±     1.474  ns/op
LinkedHashMapBenchmark2.lhsBuild             100           true  avgt   40      2626.594 ±     46.261  ns/op
LinkedHashMapBenchmark2.lhsBuild            1000           true  avgt   40     31229.980 ±    261.343  ns/op
LinkedHashMapBenchmark2.lhsBuild           10000           true  avgt   40    515878.416 ±   3385.323  ns/op
LinkedHashMapBenchmark2.lhsContainsFalse      10           true  avgt   40       794.082 ±      0.987  ns/op
LinkedHashMapBenchmark2.lhsContainsFalse     100           true  avgt   40       743.354 ±      3.808  ns/op
LinkedHashMapBenchmark2.lhsContainsFalse    1000           true  avgt   40      9481.574 ±     42.296  ns/op
LinkedHashMapBenchmark2.lhsContainsFalse   10000           true  avgt   40     178484.693 ±   235.924  ns/op
LinkedHashMapBenchmark2.lhsContainsTrue       10           true  avgt   40        86.067 ±      0.181  ns/op
LinkedHashMapBenchmark2.lhsContainsTrue      100           true  avgt   40       873.667 ±      3.007  ns/op
LinkedHashMapBenchmark2.lhsContainsTrue     1000           true  avgt   40      9570.567 ±     66.776  ns/op
LinkedHashMapBenchmark2.lhsContainsTrue    10000           true  avgt   40     119420.781 ±   470.263  ns/op
LinkedHashMapBenchmark2.lhsFillColliding      10           true  avgt   40       243.952 ±        3.2  ns/op
LinkedHashMapBenchmark2.lhsFillColliding     100           true  avgt   40      7245.039 ±     54.031  ns/op
LinkedHashMapBenchmark2.lhsFillColliding    1000           true  avgt   40    286476.023 ±   1072.997  ns/op
LinkedHashMapBenchmark2.lhsFillColliding   10000           true  avgt   40  19971599.863 ±  97841.933  ns/op
LinkedHashMapBenchmark2.lhsFillRegular        10           true  avgt   40        136.601 ±     0.685  ns/op
LinkedHashMapBenchmark2.lhsFillRegular       100           true  avgt   40      2598.248 ±     38.631  ns/op
LinkedHashMapBenchmark2.lhsFillRegular      1000           true  avgt   40     30654.817 ±     63.219  ns/op
LinkedHashMapBenchmark2.lhsFillRegular     10000           true  avgt   40     523159.815 ±  1712.072  ns/op
LinkedHashMapBenchmark2.lhsIterate            10           true  avgt   40        37.178 ±      0.009  ns/op
LinkedHashMapBenchmark2.lhsIterate           100           true  avgt   40       367.921 ±      0.562  ns/op
LinkedHashMapBenchmark2.lhsIterate          1000           true  avgt   40      3696.711 ±     11.559  ns/op
LinkedHashMapBenchmark2.lhsIterate         10000           true  avgt   40     37111.388 ±     23.572  ns/op

New linkedhashset benchmark data:
Benchmark                                 (size)  (stringsOnly)  Mode  Cnt         Score        Error  Units
LinkedHashMapBenchmark2.lhsBuild              10           true  avgt   40        99.393 ±      1.299  ns/op
LinkedHashMapBenchmark2.lhsBuild             100           true  avgt   40      1964.677 ±      5.188  ns/op
LinkedHashMapBenchmark2.lhsBuild            1000           true  avgt   40     17275.521 ±     54.087  ns/op
LinkedHashMapBenchmark2.lhsBuild           10000           true  avgt   40    197580.457 ±    650.951  ns/op
LinkedHashMapBenchmark2.lhsContainsFalse      10           true  avgt   40       609.765 ±      0.774  ns/op
LinkedHashMapBenchmark2.lhsContainsFalse     100           true  avgt   40       592.982 ±      0.460  ns/op
LinkedHashMapBenchmark2.lhsContainsFalse    1000           true  avgt   40      6121.574 ±     25.423  ns/op
LinkedHashMapBenchmark2.lhsContainsFalse   10000           true  avgt   40     75333.319 ±    273.469  ns/op
LinkedHashMapBenchmark2.lhsContainsTrue       10           true  avgt   40        56.577 ±      0.155  ns/op
LinkedHashMapBenchmark2.lhsContainsTrue      100           true  avgt   40       576.741 ±      0.224  ns/op
LinkedHashMapBenchmark2.lhsContainsTrue     1000           true  avgt   40      6329.808 ±     28.641  ns/op
LinkedHashMapBenchmark2.lhsContainsTrue    10000           true  avgt   40     66922.317 ±    232.675  ns/op
LinkedHashMapBenchmark2.lhsFillColliding      10           true  avgt   40       216.958 ±      1.042  ns/op
LinkedHashMapBenchmark2.lhsFillColliding     100           true  avgt   40      6858.672 ±     52.100  ns/op
LinkedHashMapBenchmark2.lhsFillColliding    1000           true  avgt   40    360465.883 ±   7244.140  ns/op
LinkedHashMapBenchmark2.lhsFillColliding   10000           true  avgt   40  20239064.451 ± 141196.263  ns/op
LinkedHashMapBenchmark2.lhsFillRegular        10           true  avgt   40        87.968 ±      0.234  ns/op
LinkedHashMapBenchmark2.lhsFillRegular       100           true  avgt   40      1976.888 ±      7.802  ns/op
LinkedHashMapBenchmark2.lhsFillRegular      1000           true  avgt   40     17384.031 ±    174.278  ns/op
LinkedHashMapBenchmark2.lhsFillRegular     10000           true  avgt   40    199986.822 ±   1527.214  ns/op
LinkedHashMapBenchmark2.lhsIterate            10           true  avgt   40        37.459 ±      0.155  ns/op
LinkedHashMapBenchmark2.lhsIterate           100           true  avgt   40       371.253 ±      0.761  ns/op
LinkedHashMapBenchmark2.lhsIterate          1000           true  avgt   40      3718.015 ±     14.129  ns/op
LinkedHashMapBenchmark2.lhsIterate         10000           true  avgt   40     37219.866 ±     11.121  ns/op

java linkedhashset benchmark data:
Benchmark                                     (size)  (stringsOnly)  Mode  Cnt         Score        Error  Units
LinkedHashMapBenchmark2.javalhsBuild              10           true  avgt   40       175.234 ±      0.556  ns/op
LinkedHashMapBenchmark2.javalhsBuild             100           true  avgt   40      1825.539 ±     11.813  ns/op
LinkedHashMapBenchmark2.javalhsBuild            1000           true  avgt   40     19026.942 ±     53.608  ns/op
LinkedHashMapBenchmark2.javalhsBuild           10000           true  avgt   40    159324.316 ±   1181.422  ns/op
LinkedHashMapBenchmark2.javalhsContainsFalse      10           true  avgt   40       550.725 ±      0.449  ns/op
LinkedHashMapBenchmark2.javalhsContainsFalse     100           true  avgt   40       539.374 ±      2.675  ns/op
LinkedHashMapBenchmark2.javalhsContainsFalse    1000           true  avgt   40      5767.066 ±      5.625  ns/op
LinkedHashMapBenchmark2.javalhsContainsFalse   10000           true  avgt   40     92984.208 ±    182.648  ns/op
LinkedHashMapBenchmark2.javalhsContainsTrue       10           true  avgt   40        52.468 ±      0.056  ns/op
LinkedHashMapBenchmark2.javalhsContainsTrue      100           true  avgt   40       541.565 ±      8.932  ns/op
LinkedHashMapBenchmark2.javalhsContainsTrue     1000           true  avgt   40      5707.696 ±      2.620  ns/op
LinkedHashMapBenchmark2.javalhsContainsTrue    10000           true  avgt   40     76351.601 ±    325.104  ns/op
LinkedHashMapBenchmark2.javalhsFillColliding      10           true  avgt   40       286.772 ±      0.835  ns/op
LinkedHashMapBenchmark2.javalhsFillColliding     100           true  avgt   40     16846.728 ±    102.928  ns/op
LinkedHashMapBenchmark2.javalhsFillColliding    1000           true  avgt   40    656752.211 ±   2828.982  ns/op
LinkedHashMapBenchmark2.javalhsFillColliding   10000           true  avgt   40  36264073.542 ± 314451.880  ns/op
LinkedHashMapBenchmark2.javalhsFillRegular        10           true  avgt   40       170.564 ±      0.620  ns/op
LinkedHashMapBenchmark2.javalhsFillRegular       100           true  avgt   40      2514.922 ±     17.280  ns/op
LinkedHashMapBenchmark2.javalhsFillRegular      1000           true  avgt   40     23813.883 ±    121.429  ns/op
LinkedHashMapBenchmark2.javalhsFillRegular     10000           true  avgt   40    243042.253 ±   1325.699  ns/op
LinkedHashMapBenchmark2.javalhsIterate            10           true  avgt   40        53.700 ±      0.186  ns/op
LinkedHashMapBenchmark2.javalhsIterate           100           true  avgt   40       532.121 ±      2.172  ns/op
LinkedHashMapBenchmark2.javalhsIterate          1000           true  avgt   40      4523.352 ±     10.762  ns/op
LinkedHashMapBenchmark2.javalhsIterate         10000           true  avgt   40     48223.681 ±    604.727  ns/op

@liang3zy22 liang3zy22 marked this pull request as ready for review November 25, 2022 00:37
@lrytz
Copy link
Member

lrytz commented Nov 25, 2022

Thank you @liang3zy22! I started reviewing this PR, it's looking good. I haven't finished yet though. I pushed two commits to the branch of this PR.

From looking at the numbers you posted, the performance improvements are really good.

As far as I can tell, there are optimizations in HashMap that could still be migrated to LinkedHashMap

  • overrides like hashCode, subtractAll, filterInPlace, getOrElseUpdate, getOrElse
  • in immutable.HashMap, could do the same optimizations in concat / addAll / remvovedAll as done for mutable.HashMap

The PR currently overrides the same operation that the old LinkedHashMap did already, which seems good to me. More optimizations can come later (if binary compatible).

Serialization (serializeTo) is similar to what it was in HashTalbe, but not the same (HashTable serializes more properties like loadFactor, seedValue), so not compatible. Maybe we need to make it compatible, or otherwise probably switch to the collection's default serialization.

@liang3zy22
Copy link
Contributor Author

liang3zy22 commented Nov 26, 2022 •
edited

Thanks for review, @lrytz ! Yes, I only overrides same operation as old LinkedHashMap for binary compatibility.
The HashTable trait is used in scala.collection.parallel library. So I don't know if it can be deleted.

@SethTisue SethTisue marked this pull request as draft November 27, 2022 15:52
@lrytz lrytz force-pushed the collectionbug12369 branch 2 times, most recently from 152ae6b to a238683 Compare November 28, 2022 13:24
@lrytz lrytz marked this pull request as ready for review November 28, 2022 14:41
@lrytz
Copy link
Member

lrytz commented Nov 28, 2022

I finished looking at the code changes, it's looking good to me.

HashTable trait is used in scala.collection.parallel library

Thank you, I missed that dependency. I restored HashTable.

Serialization ... is ... not compatible

When looking why SerializationStabilityTest didn't fail, I realized that LinkedHashMap/Set already extend the collection's DefaultSerializable, so the default serialization proxy was used already before this PR. The serialization-related code was unused, so it can simply be removed (latest commit).

@SethTisue
Copy link
Member

(Let's run the community build on this: after we think it's in final form, but before actual merge.)

@liang3zy22
Copy link
Contributor Author

Yeah, don't know how scala serialization work then written unused codes(just know that writeReplace function needed to be overridden).

@SethTisue SethTisue self-assigned this Nov 30, 2022
@SethTisue
Copy link
Member

SethTisue commented Nov 30, 2022 •
edited

community build run on 2.13.11-bin-89f3de5-SNAPSHOT: https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/4149/

also, @lrytz when we spoke today you mentioned doing some rebasing here, but I'm not seeing a need for it? your individual commits seem coherent?

@lrytz
Copy link
Member

lrytz commented Dec 1, 2022

OK, we can leave the commits as they are. Or squash the first two.

@SethTisue maybe you can take a look at the community build log, it's not clear to me what to make of it.

@liang3zy22 @SethTisue
new LinkedHashMap/LinkedHashSet implementation
077f714 
The mutable HashMap/HashSet has been rewroten and the
performance is better. So rewrote the LinkedHashMap and
LinkedHashSet also to improve performance. The detailed
data can be seen in the PR.
Most codes are same with HashMap/HashSet but some are
different:
1. To keep binary compatibility, only api in old solution
are updated.The two class LinkedHashMap/LinkedHashSet
still don't have parameters. hashcode can't be realized since
it needs a new iterator which will break binary compatibility.
2. Add specific method to handle the order when adding/removing
the entry.
3. other minor changes.

Signed-off-by: Liang Yan <ckgppl_yan@sina.cn>
@SethTisue SethTisue removed their assignment Dec 2, 2022
@SethTisue
Copy link
Member

SethTisue commented Dec 2, 2022 •
edited

I squashed those two commits.

Community build is green at https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/4151/ (the metals failure is known to happen with Scala -SNAPSHOTs)

@lrytz
LinkedHashMap/LinkedHashSet: review cleanups
c221509 
* Add `LinkedHashSet.add` override
* Mark the private[collection] HashTable as not used
@lrytz
Remove unused serialization-related code
d07b344 
`LinkedHashMap/Set` extend `DefaultSerializable`, so instances
are not directly serialized, the `DefaultSerializationProxy` is used
instead. Serialization-related code in `LinkedHashMap/Set` is unused.
@lrytz lrytz enabled auto-merge December 2, 2022 10:14
@lrytz lrytz merged commit 13a2976 into scala:2.13.x Dec 2, 2022
@som-snytt
Copy link
Contributor

In honor of Irene Cara, someone should play "What a Feeling!" 🎉

@liang3zy22
Copy link
Contributor Author

It's the first time I heard about this song, I can't do it.

@liang3zy22 liang3zy22 deleted the collectionbug12369 branch December 2, 2022 13:03
@lrytz lrytz mentioned this pull request Jan 25, 2023
Merged
@SethTisue SethTisue changed the title new LinkedHashMap/LinkedHashSet implementation new LinkedHashMap/LinkedHashSet implementation May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lrytz lrytz lrytz approved these changes

Assignees
No one assigned
Labels
library:collections PRs involving changes to the standard collection library release-notes worth highlighting in next release notes
Projects
None yet
Milestone
2.13.11
5 participants
@liang3zy22 @lrytz @SethTisue @som-snytt @scala-jenkins