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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slow performance when parsing file with no EJS code in the file. #227

Closed
andersjanmyr opened this issue May 2, 2023 · 9 comments
Closed

Comments

@andersjanmyr
Copy link

andersjanmyr commented May 2, 2023

When parsing a file with no EJS in, performance degrades substantially. The performance improves the closer to the end of the file the EJS code is located.
For a 60k file (600 lines with 100 chars) with no EJS, performance is around 1400ms.
When I add <%= it %> at the end of the file (line 600), performance is1ms
When I add <%= it %> at the middle of the file (line 300), performance is360ms

To Reproduce

  1. Create a large file with no EJS. (My test file is a 60k text file with 600 lines with 100 chars each.)
  2. Eta.render(body, { dummy: "yes" });
  3. Estimated time ~1400ms
  4. Add <%= it %> at end of file.
  5. Estimated time 1ms.

Package & Environment Details

  • OSX 12.6.2
  • Node v18.15.0

Test case

import assert from "assert";
import * as Eta from "eta";
import fs from "fs";

const body = fs.readFileSync("./dummy.txt", "utf8");
describe("metrics", function () {
  it("eta", function () {
    const start = Date.now();
    const text = Eta.render(body, { dummy: "yes" });
    const end = Date.now();
    console.log("eta " + (end - start));
    assert(text);
  });
});

@nebrelbug
Copy link
Collaborator

@andersjanmyr wow, this is fascinating. Tbh, I have no idea what the cause of this bug is.

@nebrelbug
Copy link
Collaborator

It seems that str.slice(0, str.length), which is called when there are no tags in the template, may just be significantly slower than RegExp capture. I'll look into this further.

@andersjanmyr
Copy link
Author

My hack to fix the performance is to append an <%= "" %> to the input.

@nebrelbug
Copy link
Collaborator

@andersjanmyr thanks for the tip. I'll see if I can fix this in Eta v3.

@joshcancode
Copy link
Contributor

joshcancode commented May 13, 2023

It seems the RegExp that checks for opening tags is the culprit. The expression consistently takes ~1000ms to run though a 60k file with no tags.

I added a check in the parse function to return if the file has no opening or closing tags, which cut down execution time to ~1ms.

Right after the "pushString" function declaration:

if (!str.includes(config.tags[0]) || !str.includes(config.tags[1])) {
  pushString(str);
  return buffer;
}

@nebrelbug
Copy link
Collaborator

@joshcancode you're right, it seems it was the RegExp. I just tried a change to the RegExp itself, and it seems to resolve the problem (should work if there's a tag in the middle of the file too.)

I pushed my code to https://github.com/eta-dev/eta/tree/fix-issue-227. Do you have any feedback?

@andersjanmyr
Copy link
Author

I can confirm that the fix works. I tried to add a test case for it but, somehow the timeout (10 ms) doesn't seem to have any effect, so it doesn't actually fail. :( jest.test docs

// test/parse.spec.ts
  const largeChunkWithNoTags = "A".repeat(64 * 1024);
  test("parses large chunk with no tags with good performance", () => {
    parse(largeChunkWithNoTags, config);
  }, 10);

@joshcancode
Copy link
Contributor

I pushed my code to https://github.com/eta-dev/eta/tree/fix-issue-227. Do you have any feedback?

Looks good to me! Consistently fast processing times, only a couple of ms now.

I can confirm that the fix works. I tried to add a test case for it but, somehow the timeout (10 ms) doesn't seem to have any effect, so it doesn't actually fail. :( jest.test docs

I'm not too familiar with this library unfortunately :( I'm not sure what could be causing that.

@nebrelbug
Copy link
Collaborator

@andersjanmyr @joshcancode just merged the fix and released in https://github.com/eta-dev/eta/releases/tag/v2.2.0!

If you come up with a way to test this, feel free to submit a PR.

Thanks for your help & feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants