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

Improve performance by using for-of loop instead of iterate functions #79

Merged
merged 7 commits into from
Oct 5, 2018

Conversation

yhatt
Copy link
Member

@yhatt yhatt commented Oct 4, 2018

Use for-of loop instead of iterate functions like Array.forEach() or Array.reduce(). It receives better performance and readability of code.

Motivations

Marpit internal plugins traverse the parsed tokens from Markdown many times. We were always used JavaScript iterate functions because of restricted by ESLint rule from Airbnb. However, for-of statement has better performance.

Marpit has targeted to modern environments. (Node >= 6 LTS, Modern browser) Airbnb says that its polyfill (regenerator-runtime) is too heavyweight, but our target environments are not required polyfill. Thus, we have allowed for-of by overridden no-restricted-syntax ESLint rule .

One more motivation is the readability of code. Array.reduce() had well used to rearrenge tokens, but they have not-intuitive codes. Using Array.push() to the empty array looks like better than Array.reduce().

It is not means that the all of iterate function are disallowed. We're still using iterate funcs to the place that has less impact of performance and expects to lost code readabilty by replacing to for-of.

Benchmark

We have measured the time of 1000 conversions from example of uncover theme. Averages of 20 trials are below:

Node v8 Node v6
Iterate functions 2079.95ms 2778.75ms
for-of 1211.00ms 2008.5ms
const request = require('request')
const { Marpit } = require('./lib/')

const average = arr => arr.reduce((p, c) => p + c, 0) / arr.length
const marpit = new Marpit()

request.get(
  'https://gist.githubusercontent.com/yhatt/a352e6087c2d5185113b9143f330fb68/raw/ca3ca25b3a022ccaae3836c0464b02693230dcef/_uncover.md',
  (_, __, body) => {
    const tries = []

    for (let t = -5; t < 20; t += 1) {
      const start = new Date().getTime()

      for (let i = 0; i < 1000; i += 1) marpit.render(body)
      const end = new Date().getTime() - start

      if (t >= 0) tries.push(end)
      console.log(`Try ${t >= 0 ? t + 1 : 'for idling'}: ${end}ms`)
    }

    console.log(`Average: ${average(tries)}ms`)
  }
)

@yhatt
Copy link
Member Author

yhatt commented Oct 5, 2018

UPDATE: Add 5 times idling in benchmark code. We have re-measured and it seems to be about 40-70% faster.

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

Successfully merging this pull request may close these issues.

1 participant