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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Equality check between two sparse arrays iterate over holes to compare them #11055

Closed
dubzzz opened this issue Feb 5, 2021 · 4 comments
Closed

Comments

@dubzzz
Copy link
Contributor

dubzzz commented Feb 5, 2021

馃悰 Bug Report

Checking the equality of two sparse arrays seems to iterate over all the cells of each of those (as soon as they have the same size).

By looking at the code I found that there was a special case for sparse arrays: https://github.com/facebook/jest/blob/0a9e77d5e75adc87f8d7497ec30472c661c6fc11/packages/expect/src/utils.ts#L334

But its seems that the special case only checks hole versus undefined case as it finally calls equals that will iterate over all indexes one after the other: https://github.com/facebook/jest/blob/0a9e77d5e75adc87f8d7497ec30472c661c6fc11/packages/expect/src/jasmineUtils.ts#L153

I am wondering if we could not have it faster. Actually for sparse arrays iterating over the keys and not all the indexes should be enough to confirm equality, no?

To Reproduce

Steps to reproduce the behavior:

expect(Array(100000000)).toEqual(Array(100000000)); // or longer

Expected behavior

Ends quickly: lengths are equal, keys are equal, values associated to keys are equal.

envinfo

  System:
    OS: Windows 10 10.0.19042
    CPU: (8) x64 Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
  Binaries:
    Node: 14.15.4 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.5 - ~\.yarn\bin\yarn.CMD
    npm: 6.14.10 - C:\Program Files\nodejs\npm.CMD
  npmPackages:
    jest: ^26.1.0 => 26.6.3

Context

I am currently working into adding generators for sparse arrays in fast-check (see dubzzz/fast-check#1447). My plan was to provide such helper built-in as I believed it could help to detect some bugs...

@dubzzz
Copy link
Contributor Author

dubzzz commented Oct 12, 2022

@SimenB If I'm not wrong I think the only remaining part for this issue was the fact that the computation of the stringified version of an Array(100000000) is taking lots of time in Jest.

Indeed Jest creates something like [,,,...,,,]. But it sounds expected if we want this string version to be printed on user side. On fast-check when arrays are pretty sparse (with many holes) I print something like Object.assign(Array(100000000), {5: 'value for 5'}) but it's not perfect as it seems less readable than a [,,,....,,,,].

Not sure 100% but if I'm not wrong the stringified version is computed even if the assertion passes. But I'm really not sure about that last statement.

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Oct 12, 2023
Copy link

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 11, 2023
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants