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

feat: performance improvements #18

Merged
merged 7 commits into from Aug 7, 2019

Conversation

@wooorm
Copy link
Contributor

commented Aug 1, 2019

Hi folks! 馃憢

@zeke mentioned that y鈥檃ll were looking to improve performance in hubdown so I thought I鈥檇 give my suggestions in the form of a PR!

wooorm added some commits Aug 1, 2019

const hasha = require('hasha')
const stableStringify = require('json-stable-stringify')

// Create processor once, if possible.
const defaultProcessor = createProcessor()

This comment has been minimized.

Copy link
@wooorm

wooorm Aug 1, 2019

Author Contributor

Memoizing the processor will help a bit

.use(slug)
.use(autolinkHeadings, { behavior: 'wrap' })
.use(highlight)
.use(html)

This comment has been minimized.

Copy link
@wooorm

wooorm Aug 1, 2019

Author Contributor

Switched this up to use unified/remark/rehype directly, it鈥檚 a bit more explicit and needs less dependencies in memory


const md = await pify(renderer.process)(content)
Object.assign(data, { content: md.contents })
const file = await processor.process(content)

This comment has been minimized.

Copy link
@wooorm

wooorm Aug 1, 2019

Author Contributor

pify wasn鈥檛 needed, unified/remark support promises by default.

"remark-gemoji-to-emoji": "^1.1.0",
"remark-highlight.js": "^5.0.0",
"remark-html": "^6.0.1",
"remark-inline-links": "^3.0.4",

This comment has been minimized.

Copy link
@wooorm

wooorm Aug 1, 2019

Author Contributor

remark already supports references and definitions. remark-inline-links isn鈥檛 needed here, as it changes the markdown syntax tree to replace references and definitions with normal links. But because we鈥檙e going to HTML here everything gets changed to <a>s anyway

@juliangruber

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

This branch takes another 3s build time off of the project I'm working on! 馃巻

@HashimotoYT HashimotoYT self-requested a review Aug 6, 2019

@zeke

zeke approved these changes Aug 7, 2019

Copy link
Member

left a comment

This all looks great to me! I just to want to test it out in our app before landing.

@HashimotoYT
Copy link
Member

left a comment

馃ぞ鈥嶁檧锔, electron-i18n test suite passed.

@HashimotoYT HashimotoYT changed the title Improvements feat: performance improvements Aug 7, 2019

@zeke zeke merged commit a0f9aca into electron:master Aug 7, 2019

3 checks passed

Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
ci/circleci: test Your tests passed on CircleCI!
Details
@electron-bot

This comment has been minimized.

Copy link

commented Aug 7, 2019

馃帀 This PR is included in version 2.4.0 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can鈥檛 perform that action at this time.