Skip to content

Improve performance by a lot (although quite a dirty patch)#50

Merged
soasme merged 2 commits intomasterfrom
unknown repository
Mar 5, 2021
Merged

Improve performance by a lot (although quite a dirty patch)#50
soasme merged 2 commits intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Mar 5, 2021

Right now nim-markdown recompiles all regular expressions on EACH call to any method/proc, so that if some proc is called 100 times, re module (which uses pcre) has to recompile the same regular expression 100 times - see documentation of https://nim-lang.org/docs/re.html#re%2Cstring

In the long run it would be better to actually define variables with precompiled expressions like:

let someNameForMyRe = re"abc"

proc test = 
  let a = match("helloabc", someNameForMyRe)

but this patch will work for now.

For a simple MD document (taken from Casa) - https://gist.github.com/Yardanico/3ed52233c29b6328bedce30a9f332c3a

I got a ~8x time improvement - from 27.5ms to 3ms:

# Without patch
name ............................... min time      avg time    std dv   runs
test .............................. 26.906 ms     27.842 ms    ±0.512   x179
# With patch
name ............................... min time      avg time    std dv   runs
test ............................... 2.936 ms      3.022 ms    ±0.086  x1000

This is already much better, but I think that we can make it even faster :)

@ghost
Copy link
Author

ghost commented Mar 5, 2021

Another benchmark (with https://github.com/treeform/benchy):

import benchy
import markdown

let data = "# Hello world"

timeIt "test":
  let a = markdown(data)
  keep(a)

With the patch:

name ............................... min time      avg time    std dv   runs
test ............................... 0.019 ms      0.024 ms    ±0.004  x1000

Without it:

name ............................... min time      avg time    std dv   runs
test ............................... 0.326 ms      0.345 ms    ±0.007  x1000

14x time improvement for a markdown hello-world (from 0.35ms to 0.025ms)!

@ghost
Copy link
Author

ghost commented Mar 5, 2021

By the way, I checked and all tests pass just fine.

@ghost ghost changed the title Hotfix performance (quite a dirty hack) Improve performance by a lot (although quite a dirty patch) Mar 5, 2021

from markdownpkg/entities import htmlEntityToUtf8

var precompiledExp {.threadvar.}: Table[string, re.Regex]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The {.threadvar.} annotation makes this caching thread-safe (each different thread will have to compile regular expressions by itself, but I think that this is fine)

@ghost
Copy link
Author

ghost commented Mar 5, 2021

Related: #48

@soasme
Copy link
Owner

soasme commented Mar 5, 2021

Awesome. I like the solution!

@soasme soasme merged commit a27a1cb into soasme:master Mar 5, 2021
@ghost ghost mentioned this pull request Mar 5, 2021
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