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

馃悰 Bug: diff@5.0.0 has a performance bug #5144

Closed
4 tasks done
jedwards1211 opened this issue May 16, 2024 · 1 comment
Closed
4 tasks done

馃悰 Bug: diff@5.0.0 has a performance bug #5144

jedwards1211 opened this issue May 16, 2024 · 1 comment
Labels
duplicate been there, done that, got the t-shirt... type: bug a defect, confirmed by a maintainer

Comments

@jedwards1211
Copy link

jedwards1211 commented May 16, 2024

Bug Report Checklist

  • I have read and agree to Mocha's Code of Conduct and Contributing Guidelines
  • I have searched for related issues and issues with the faq label, but none matched my issue.
  • I have 'smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, my usage of Mocha, or Mocha itself.
  • I want to provide a PR to resolve this

Expected

Diff performs well even on large inputs

Actual

diff@5.0.0 has catastrophic performance in certain cases (see kpdecker/jsdiff#239).

On my machine, the diff in this example takes ~45 seconds. If I pin to diff@5.2.0, it's almost instantaneous.

It takes some fairly custom settings to trigger this issue in Mocha, but still, you might as well upgrade to diff@5.2.0.

Minimal, Reproducible Example

.mocharc.js

module.exports = {
  spec: ["test.js"],
  "inline-diffs": false,
  reporter: "spec",
  "reporter-option": ["maxDiffSize=525000"],
};

test.js

const { expect } = require("chai");
const { it } = require("mocha");

// make a string of 25k lines of random numbers:
text1 = new Array(25000)
  .fill(0)
  .map((n) =>
    new Array(20)
      .fill(1)
      .map((n) => Math.floor(Math.random() * 10))
      .join("")
  )
  .join("\n");
// grab the first 10,000 characters:
text2 = text1.slice(0, 10000);

it("diff test", () => {
  expect(text1).to.deep.equal(text2);
});

Versions

mocha: 10.4.0
chai: 4.4.1
diff: 5.0.0
node: 20.10.0

Additional Info

No response

@jedwards1211 jedwards1211 added status: in triage a maintainer should (re-)triage (review) this issue type: bug a defect, confirmed by a maintainer labels May 16, 2024
@JoshuaKGoldberg JoshuaKGoldberg changed the title 馃悰 Bug: diff@5.0.0 has a major performance bug 馃悰 Bug: Add ^ diff@5.0.0 has a performance bug May 25, 2024
@JoshuaKGoldberg JoshuaKGoldberg changed the title 馃悰 Bug: Add ^ diff@5.0.0 has a performance bug 馃悰 Bug: diff@5.0.0 has a performance bug May 25, 2024
@JoshuaKGoldberg
Copy link
Member

Fun. The root issue on Mocha's side is that it pins diff to a specific major version. So this is really a subset of #5114. Marking as a duplicate of that one - thanks for filing!

@JoshuaKGoldberg JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale May 25, 2024
@JoshuaKGoldberg JoshuaKGoldberg added duplicate been there, done that, got the t-shirt... and removed status: in triage a maintainer should (re-)triage (review) this issue labels May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate been there, done that, got the t-shirt... type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

2 participants