Convert the `Dict`-implementation to use a `Map` internally #19051

Snuffleupagus · 2024-11-16T11:01:09Z

With all the recent work happening under https://bugzilla.mozilla.org/show_bug.cgi?id=1851662, the performance of Map is already good enough that I believe that we should now be able to utilize it in the Dict-class without problem.

This patch was tested in Firefox Nightly, specifically build https://hg.mozilla.org/mozilla-central/rev/6c508a387477e3b72db913a9e1761e9a433d06a2, with the following manifest file:

[
  {
    "id": "tracemonkey-eq",
    "file": "pdfs/tracemonkey.pdf",
    "md5": "9a192d8b1a7dc652a19835f6f08098bd",
    "rounds": 100,
    "type": "eq"
  },
  {
    "id": "issue2618",
    "file": "pdfs/issue2618.pdf",
    "md5": "2c554a99a52288ca1a44a422eeafb8fb",
    "rounds": 100,
    "type": "eq"
  }
]

which gave the following results, indicating no significant regression, when comparing this patch against the master branch:

Overall

-- Grouped By browser, pdf, stat --
browser | pdf            | stat         | Count | Baseline(ms) | Current(ms) | +/- |    %  | Result(P<.05)
------- | -------------- | ------------ | ----- | ------------ | ----------- | --- | ----- | -------------
firefox | issue2618      | Overall      |   100 |          678 |         678 |   0 |  0.04 |
firefox | issue2618      | Page Request |   100 |            1 |           1 |   0 | -3.88 |
firefox | issue2618      | Rendering    |   100 |          677 |         677 |   0 |  0.05 |
firefox | tracemonkey-eq | Overall      |  1400 |           35 |          36 |   0 |  0.96 |
firefox | tracemonkey-eq | Page Request |  1400 |            1 |           1 |   0 | -8.08 |
firefox | tracemonkey-eq | Rendering    |  1400 |           34 |          35 |   0 |  1.26 |

Page-specific

-- Grouped By browser, pdf, page, stat --
browser | pdf            | page | stat         | Count | Baseline(ms) | Current(ms) | +/- |     %  | Result(P<.05)
------- | -------------- | ---- | ------------ | ----- | ------------ | ----------- | --- | ------ | -------------
firefox | issue2618      | 0    | Overall      |   100 |          678 |         678 |   0 |   0.04 |
firefox | issue2618      | 0    | Page Request |   100 |            1 |           1 |   0 |  -3.88 |
firefox | issue2618      | 0    | Rendering    |   100 |          677 |         677 |   0 |   0.05 |
firefox | tracemonkey-eq | 0    | Overall      |   100 |           23 |          24 |   0 |   1.24 |
firefox | tracemonkey-eq | 0    | Page Request |   100 |            1 |           1 |   0 |  19.77 |
firefox | tracemonkey-eq | 0    | Rendering    |   100 |           23 |          23 |   0 |   0.40 |
firefox | tracemonkey-eq | 1    | Overall      |   100 |           32 |          32 |  -1 |  -1.89 |
firefox | tracemonkey-eq | 1    | Page Request |   100 |            1 |           1 |   0 | -28.13 |
firefox | tracemonkey-eq | 1    | Rendering    |   100 |           31 |          31 |   0 |  -0.77 |
firefox | tracemonkey-eq | 2    | Overall      |   100 |           17 |          18 |   1 |   4.60 |
firefox | tracemonkey-eq | 2    | Page Request |   100 |            1 |           1 |   0 |  23.53 |        slower
firefox | tracemonkey-eq | 2    | Rendering    |   100 |           17 |          17 |   1 |   3.71 |
firefox | tracemonkey-eq | 3    | Overall      |   100 |           23 |          24 |   0 |   1.71 |
firefox | tracemonkey-eq | 3    | Page Request |   100 |            1 |           1 |   0 |   7.79 |
firefox | tracemonkey-eq | 3    | Rendering    |   100 |           23 |          23 |   0 |   1.55 |
firefox | tracemonkey-eq | 4    | Overall      |   100 |           31 |          31 |   1 |   2.49 |
firefox | tracemonkey-eq | 4    | Page Request |   100 |            1 |           1 |   0 |  48.96 |
firefox | tracemonkey-eq | 4    | Rendering    |   100 |           30 |          30 |   0 |   1.05 |
firefox | tracemonkey-eq | 5    | Overall      |   100 |           31 |          30 |  -1 |  -2.42 |
firefox | tracemonkey-eq | 5    | Page Request |   100 |            2 |           1 |  -1 | -49.33 |
firefox | tracemonkey-eq | 5    | Rendering    |   100 |           29 |          29 |   0 |  -0.03 |
firefox | tracemonkey-eq | 6    | Overall      |   100 |           27 |          27 |   0 |   1.81 |
firefox | tracemonkey-eq | 6    | Page Request |   100 |            1 |           1 |   0 |   4.94 |
firefox | tracemonkey-eq | 6    | Rendering    |   100 |           26 |          27 |   0 |   1.68 |
firefox | tracemonkey-eq | 7    | Overall      |   100 |           26 |          26 |   1 |   3.13 |
firefox | tracemonkey-eq | 7    | Page Request |   100 |            1 |           1 |   0 |   6.98 |
firefox | tracemonkey-eq | 7    | Rendering    |   100 |           25 |          25 |   1 |   2.92 |
firefox | tracemonkey-eq | 8    | Overall      |   100 |           25 |          26 |   1 |   5.16 |
firefox | tracemonkey-eq | 8    | Page Request |   100 |            1 |           1 |  -1 | -41.84 |
firefox | tracemonkey-eq | 8    | Rendering    |   100 |           23 |          25 |   2 |   8.19 |
firefox | tracemonkey-eq | 9    | Overall      |   100 |           33 |          33 |   0 |   0.03 |
firefox | tracemonkey-eq | 9    | Page Request |   100 |            1 |           1 |   0 |   0.79 |
firefox | tracemonkey-eq | 9    | Rendering    |   100 |           32 |          32 |   0 |  -0.10 |
firefox | tracemonkey-eq | 10   | Overall      |   100 |          144 |         144 |   1 |   0.52 |
firefox | tracemonkey-eq | 10   | Page Request |   100 |            2 |           1 |  -1 | -43.52 |
firefox | tracemonkey-eq | 10   | Rendering    |   100 |          141 |         143 |   2 |   1.18 |
firefox | tracemonkey-eq | 11   | Overall      |   100 |           24 |          25 |   1 |   2.51 |
firefox | tracemonkey-eq | 11   | Page Request |   100 |            1 |           1 |   0 |  -4.71 |
firefox | tracemonkey-eq | 11   | Rendering    |   100 |           23 |          24 |   1 |   2.78 |
firefox | tracemonkey-eq | 12   | Overall      |   100 |           40 |          39 |  -1 |  -1.67 |
firefox | tracemonkey-eq | 12   | Page Request |   100 |            1 |           1 |   0 |  14.71 |
firefox | tracemonkey-eq | 12   | Rendering    |   100 |           39 |          38 |  -1 |  -1.98 |
firefox | tracemonkey-eq | 13   | Overall      |   100 |           19 |          20 |   1 |   3.09 |
firefox | tracemonkey-eq | 13   | Page Request |   100 |            1 |           1 |   0 |  24.79 |
firefox | tracemonkey-eq | 13   | Rendering    |   100 |           18 |          19 |   0 |   1.70 |

Snuffleupagus · 2024-11-16T11:04:20Z

/botio test

moz-tools-bot · 2024-11-16T11:04:23Z

From: Bot.io (Windows)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/e8ec0c91d58b53e/output.txt

moz-tools-bot · 2024-11-16T11:04:23Z

From: Bot.io (Linux m4)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/1253e29d19cc8f9/output.txt

moz-tools-bot · 2024-11-16T11:35:07Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/1253e29d19cc8f9/output.txt

Total script time: 30.73 mins

Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 18

Image differences available at: http://54.241.84.105:8877/1253e29d19cc8f9/reftest-analyzer.html#web=eq.log

moz-tools-bot · 2024-11-16T11:50:43Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/e8ec0c91d58b53e/output.txt

Total script time: 46.32 mins

Unit tests: Passed
Integration Tests: FAILED
Regression tests: Passed

timvandermeij

r=me, with the comment below. Thank you for doing this!

src/core/primitives.js

- Ensure that `pdfjsTestingUtils` is available when running benchmarking, since that shouldn't be done in TESTING-mode. - Exclude the `test/stats/results/` folder from linting, since it'll contain *generated* JSON-files.

With all the recent work happening under https://bugzilla.mozilla.org/show_bug.cgi?id=1851662, the performance of `Map` is already good enough that I believe that we should now be able to utilize it in the `Dict`-class without problem. This patch was tested in Firefox Nightly, specifically build https://hg.mozilla.org/mozilla-central/rev/6c508a387477e3b72db913a9e1761e9a433d06a2, with the following manifest file: ``` [ { "id": "tracemonkey-eq", "file": "pdfs/tracemonkey.pdf", "md5": "9a192d8b1a7dc652a19835f6f08098bd", "rounds": 100, "type": "eq" }, { "id": "issue2618", "file": "pdfs/issue2618.pdf", "md5": "2c554a99a52288ca1a44a422eeafb8fb", "rounds": 100, "type": "eq" } ] ``` which gave the following results, indicating no significant regression, when comparing this patch against the `master` branch: - Overall ``` -- Grouped By browser, pdf, stat -- browser | pdf | stat | Count | Baseline(ms) | Current(ms) | +/- | % | Result(P<.05) ------- | -------------- | ------------ | ----- | ------------ | ----------- | --- | ----- | ------------- firefox | issue2618 | Overall | 100 | 678 | 678 | 0 | 0.04 | firefox | issue2618 | Page Request | 100 | 1 | 1 | 0 | -3.88 | firefox | issue2618 | Rendering | 100 | 677 | 677 | 0 | 0.05 | firefox | tracemonkey-eq | Overall | 1400 | 35 | 36 | 0 | 0.96 | firefox | tracemonkey-eq | Page Request | 1400 | 1 | 1 | 0 | -8.08 | firefox | tracemonkey-eq | Rendering | 1400 | 34 | 35 | 0 | 1.26 | ``` - Page-specific ``` -- Grouped By browser, pdf, page, stat -- browser | pdf | page | stat | Count | Baseline(ms) | Current(ms) | +/- | % | Result(P<.05) ------- | -------------- | ---- | ------------ | ----- | ------------ | ----------- | --- | ------ | ------------- firefox | issue2618 | 0 | Overall | 100 | 678 | 678 | 0 | 0.04 | firefox | issue2618 | 0 | Page Request | 100 | 1 | 1 | 0 | -3.88 | firefox | issue2618 | 0 | Rendering | 100 | 677 | 677 | 0 | 0.05 | firefox | tracemonkey-eq | 0 | Overall | 100 | 23 | 24 | 0 | 1.24 | firefox | tracemonkey-eq | 0 | Page Request | 100 | 1 | 1 | 0 | 19.77 | firefox | tracemonkey-eq | 0 | Rendering | 100 | 23 | 23 | 0 | 0.40 | firefox | tracemonkey-eq | 1 | Overall | 100 | 32 | 32 | -1 | -1.89 | firefox | tracemonkey-eq | 1 | Page Request | 100 | 1 | 1 | 0 | -28.13 | firefox | tracemonkey-eq | 1 | Rendering | 100 | 31 | 31 | 0 | -0.77 | firefox | tracemonkey-eq | 2 | Overall | 100 | 17 | 18 | 1 | 4.60 | firefox | tracemonkey-eq | 2 | Page Request | 100 | 1 | 1 | 0 | 23.53 | slower firefox | tracemonkey-eq | 2 | Rendering | 100 | 17 | 17 | 1 | 3.71 | firefox | tracemonkey-eq | 3 | Overall | 100 | 23 | 24 | 0 | 1.71 | firefox | tracemonkey-eq | 3 | Page Request | 100 | 1 | 1 | 0 | 7.79 | firefox | tracemonkey-eq | 3 | Rendering | 100 | 23 | 23 | 0 | 1.55 | firefox | tracemonkey-eq | 4 | Overall | 100 | 31 | 31 | 1 | 2.49 | firefox | tracemonkey-eq | 4 | Page Request | 100 | 1 | 1 | 0 | 48.96 | firefox | tracemonkey-eq | 4 | Rendering | 100 | 30 | 30 | 0 | 1.05 | firefox | tracemonkey-eq | 5 | Overall | 100 | 31 | 30 | -1 | -2.42 | firefox | tracemonkey-eq | 5 | Page Request | 100 | 2 | 1 | -1 | -49.33 | firefox | tracemonkey-eq | 5 | Rendering | 100 | 29 | 29 | 0 | -0.03 | firefox | tracemonkey-eq | 6 | Overall | 100 | 27 | 27 | 0 | 1.81 | firefox | tracemonkey-eq | 6 | Page Request | 100 | 1 | 1 | 0 | 4.94 | firefox | tracemonkey-eq | 6 | Rendering | 100 | 26 | 27 | 0 | 1.68 | firefox | tracemonkey-eq | 7 | Overall | 100 | 26 | 26 | 1 | 3.13 | firefox | tracemonkey-eq | 7 | Page Request | 100 | 1 | 1 | 0 | 6.98 | firefox | tracemonkey-eq | 7 | Rendering | 100 | 25 | 25 | 1 | 2.92 | firefox | tracemonkey-eq | 8 | Overall | 100 | 25 | 26 | 1 | 5.16 | firefox | tracemonkey-eq | 8 | Page Request | 100 | 1 | 1 | -1 | -41.84 | firefox | tracemonkey-eq | 8 | Rendering | 100 | 23 | 25 | 2 | 8.19 | firefox | tracemonkey-eq | 9 | Overall | 100 | 33 | 33 | 0 | 0.03 | firefox | tracemonkey-eq | 9 | Page Request | 100 | 1 | 1 | 0 | 0.79 | firefox | tracemonkey-eq | 9 | Rendering | 100 | 32 | 32 | 0 | -0.10 | firefox | tracemonkey-eq | 10 | Overall | 100 | 144 | 144 | 1 | 0.52 | firefox | tracemonkey-eq | 10 | Page Request | 100 | 2 | 1 | -1 | -43.52 | firefox | tracemonkey-eq | 10 | Rendering | 100 | 141 | 143 | 2 | 1.18 | firefox | tracemonkey-eq | 11 | Overall | 100 | 24 | 25 | 1 | 2.51 | firefox | tracemonkey-eq | 11 | Page Request | 100 | 1 | 1 | 0 | -4.71 | firefox | tracemonkey-eq | 11 | Rendering | 100 | 23 | 24 | 1 | 2.78 | firefox | tracemonkey-eq | 12 | Overall | 100 | 40 | 39 | -1 | -1.67 | firefox | tracemonkey-eq | 12 | Page Request | 100 | 1 | 1 | 0 | 14.71 | firefox | tracemonkey-eq | 12 | Rendering | 100 | 39 | 38 | -1 | -1.98 | firefox | tracemonkey-eq | 13 | Overall | 100 | 19 | 20 | 1 | 3.09 | firefox | tracemonkey-eq | 13 | Page Request | 100 | 1 | 1 | 0 | 24.79 | firefox | tracemonkey-eq | 13 | Rendering | 100 | 18 | 19 | 0 | 1.70 | ```

Snuffleupagus · 2024-11-17T11:47:41Z

/botio unittest

moz-tools-bot · 2024-11-17T11:47:44Z

From: Bot.io (Linux m4)

Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/c562f554f725eb9/output.txt

moz-tools-bot · 2024-11-17T11:47:44Z

From: Bot.io (Windows)

Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/ba42d0c1dd0871e/output.txt

moz-tools-bot · 2024-11-17T11:50:19Z

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/c562f554f725eb9/output.txt

Total script time: 2.57 mins

Unit Tests: Passed

moz-tools-bot · 2024-11-17T11:54:29Z

From: Bot.io (Windows)

Success

Full output at http://54.193.163.58:8877/ba42d0c1dd0871e/output.txt

Total script time: 6.73 mins

Unit Tests: Passed

Snuffleupagus added the core label Nov 16, 2024

Snuffleupagus force-pushed the Dict-Map branch from de8f60c to 7779f9d Compare November 16, 2024 12:30

Snuffleupagus requested a review from calixteman November 17, 2024 09:40

timvandermeij approved these changes Nov 17, 2024

View reviewed changes

src/core/primitives.js Outdated Show resolved Hide resolved

Snuffleupagus added 3 commits November 17, 2024 12:44

Fix a couple of bugs affecting benchmarking

729e319

- Ensure that `pdfjsTestingUtils` is available when running benchmarking, since that shouldn't be done in TESTING-mode. - Exclude the `test/stats/results/` folder from linting, since it'll contain *generated* JSON-files.

Replace the forEach method in Dict with "proper" iteration support

2c0cc48

Snuffleupagus force-pushed the Dict-Map branch from 7779f9d to 2c0cc48 Compare November 17, 2024 11:45

Snuffleupagus removed the request for review from calixteman November 17, 2024 11:51

Snuffleupagus merged commit bc91985 into mozilla:master Nov 17, 2024
9 checks passed

Snuffleupagus deleted the Dict-Map branch November 17, 2024 11:59

Snuffleupagus mentioned this pull request Jan 2, 2025

Use Dict iteration more (PR 19051 follow-up) #19278

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Convert the `Dict`-implementation to use a `Map` internally #19051

Convert the `Dict`-implementation to use a `Map` internally #19051

Uh oh!

Snuffleupagus commented Nov 16, 2024

Uh oh!

Snuffleupagus commented Nov 16, 2024

Uh oh!

moz-tools-bot commented Nov 16, 2024

Uh oh!

moz-tools-bot commented Nov 16, 2024

Uh oh!

moz-tools-bot commented Nov 16, 2024

Uh oh!

moz-tools-bot commented Nov 16, 2024

Uh oh!

timvandermeij left a comment •

edited

Loading

Uh oh!

Uh oh!

Snuffleupagus commented Nov 17, 2024

Uh oh!

moz-tools-bot commented Nov 17, 2024

Uh oh!

moz-tools-bot commented Nov 17, 2024

Uh oh!

moz-tools-bot commented Nov 17, 2024

Uh oh!

moz-tools-bot commented Nov 17, 2024

Uh oh!

Uh oh!

Uh oh!

Convert the Dict-implementation to use a Map internally #19051

Convert the Dict-implementation to use a Map internally #19051

Uh oh!

Conversation

Snuffleupagus commented Nov 16, 2024

Uh oh!

Snuffleupagus commented Nov 16, 2024

Uh oh!

moz-tools-bot commented Nov 16, 2024

From: Bot.io (Windows)

Received

Uh oh!

moz-tools-bot commented Nov 16, 2024

From: Bot.io (Linux m4)

Received

Uh oh!

moz-tools-bot commented Nov 16, 2024

From: Bot.io (Linux m4)

Failed

Uh oh!

moz-tools-bot commented Nov 16, 2024

From: Bot.io (Windows)

Failed

Uh oh!

timvandermeij left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Snuffleupagus commented Nov 17, 2024

Uh oh!

moz-tools-bot commented Nov 17, 2024

From: Bot.io (Linux m4)

Received

Uh oh!

moz-tools-bot commented Nov 17, 2024

From: Bot.io (Windows)

Received

Uh oh!

moz-tools-bot commented Nov 17, 2024

From: Bot.io (Linux m4)

Success

Uh oh!

moz-tools-bot commented Nov 17, 2024

From: Bot.io (Windows)

Success

Uh oh!

Uh oh!

Uh oh!

Convert the `Dict`-implementation to use a `Map` internally #19051

Convert the `Dict`-implementation to use a `Map` internally #19051

timvandermeij left a comment •

edited

Loading