redcarpet
redcarpet copied to clipboard
Make Redcarpet::Markdown#render thread safe
I ran into the same issue as #570 and I've been trying to think of a good way to fix the issue. The solution is not entirely obvious but as far as I can tell, there are a few options:
- Update the documentation to say that
Redcarpet::Markdown
is not thread safe and you should make one per thread. - Add a global rendering lock. That's what I have here. Based on benchmarks on my laptop, this has an ~2% performance hit for the single threaded case and you get no benefit from concurrency but it doesn't just crash the program.
- Split the state into the fixed state (that can be shared between threads) and the rendering state (that needs to be separate). This was the suggestion in that issue. I made a rough attempt at that but
sd_markdown->work_bufs
is used to cache memory buffers between renders. This doesn't work in the concurrent world and if we just re-create them for every render there's a noticeable performance hit on subsequent renders using the sameRedcarpet::Markdown
object (~7%). Plus the active encoding is stored in the renderer options making that also not thread safe so we'd have to split those options in two as well.
While I could probably implement option 3, I'm not sure it's possible without at least a minor performance hit. So my questions for you are
- Is this a thing you want fixed?
- Is this a thing you want fixed enough to warrant a performance hit? If so how much?
- Do you think there's value in actually making
render
concurrent? It's fairly likely this will speed up the concurrent case but slow down the sequential case.