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

eat too much cpu on 3.0.5 #54

Closed
fengmk2 opened this issue Feb 12, 2015 · 7 comments
Closed

eat too much cpu on 3.0.5 #54

fengmk2 opened this issue Feb 12, 2015 · 7 comments
Labels

Comments

@fengmk2
Copy link

fengmk2 commented Feb 12, 2015

Parse blew markdown, 3.0.5 use 0m20.459s, and 3.0.4 use 0m2.981s

[@addyosmani](https://github.com/addyosmani),
[@borismus](https://github.com/borismus),
[@carsonmcdonald](https://github.com/carsonmcdonald),
[@chriseppstein](https://github.com/chriseppstein),
[@danwrong](https://github.com/danwrong),
[@davidmaxwaterman](https://github.com/davidmaxwaterman),
[@desandro](https://github.com/desandro),
[@hemanth](https://github.com/hemanth),
[@isaacs](https://github.com/isaacs),
[@josh](https://github.com/josh),
[@jrburke](https://github.com/jrburke),
[@marcelombc](https://github.com/marcelombc),
[@marcooliveira](https://github.com/marcooliveira),
[@mklabs](https://github.com/mklabs),
[@MrDHat](https://github.com/MrDHat),
[@necolas](https://github.com/necolas),
[@paulirish](https://github.com/paulirish),
[@richo](https://github.com/richo),
[@rvagg](https://github.com/rvagg),
[@sindresorhus](https://github.com/sindresorhus),
[@SlexAxton](https://github.com/SlexAxton),
[@sstephenson](https://github.com/sstephenson),
[@svnlto](https://github.com/svnlto),
[@tomdale](https://github.com/tomdale),
[@uzquiano](https://github.com/uzquiano),

repeat codes:

var MarkdownIt = require('markdown-it');

    var md = new MarkdownIt({
      html: true,
      linkify: true,
    });
    var readme = fs.readFileSync(path.join(fixtures, 'eat-cpu.md'), 'utf8');
    md.render(readme);
@fengmk2
Copy link
Author

fengmk2 commented Feb 12, 2015

/cc @puzrin this is rush!

@puzrin puzrin added the bug label Feb 12, 2015
@puzrin
Copy link
Member

puzrin commented Feb 12, 2015

Problem is in this line change 8ca0b5b#diff-f4f9b75e4799aa5566d488b8f480de40L37

Change looks correct, but i don't see any broken tests on revert.

/cc @rlidwka , take a look please. I don't see check for ]:. Also, we should check what happens with recursion on real references, because list can be long, and that should not fall under our nesting limit..

@puzrin
Copy link
Member

puzrin commented Feb 12, 2015

https://github.com/markdown-it/markdown-it/blob/master/lib/parser_block.js#L18

[reference] seems to be a mistake. After playing with official dingus & digging spec, i didn't found a way to do such interrupt. This rule cause heavy recursion, trashing everything.

@fengmk2 if you can't wait at all - remove nested array from that line. Will publish fix a bit later, when @rlidwka appears online.

@rlidwka
Copy link
Member

rlidwka commented Feb 12, 2015

Fixed in refs branch.

It was an optimization attempt gone south. If we have a long list of references (like 533b5c8), this parser would call getLines for each reference until the end of the paragraph. So first call would be getLines(1, 50), second call would be getLines(2, 50), etc. So I thought, it would be nice to optimize it a bit. It didn't go so well.

Anyway, the immediate solution would be to remove references from there.

And we have a benchmark that performs 3x times slower than CommonMark (in theory, their parser is linear on that input, and ours isn't), but it might be left for future discussions.

@fengmk2
Copy link
Author

fengmk2 commented Feb 12, 2015

I use 3.0.4 right now.

@puzrin
Copy link
Member

puzrin commented Feb 12, 2015

Fixed in published 3.0.6

@puzrin puzrin closed this as completed Feb 12, 2015
@fengmk2
Copy link
Author

fengmk2 commented Feb 13, 2015

cool! Thanks @puzrin @rlidwka

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

No branches or pull requests

3 participants