Replace empty strings with a global constant

[ashmaroli]

This to serve as a PoC that we can avoid creating numerous empty string objects by using a single frozen constant defined directly under the Jekyll namespace

Theoretical Benefit:

• lesser allocation => lower memory fingerprint

Possible drawbacks:

• Affects readability

/cc @pathawks @envygeeks @parkr

[pathawks]

Are you sure this actually results in fewer allocations?

I don’t know about Ruby, but in Java this would not make any difference because of the string pool.

Because this affects readability so much, I want to know for certain that this actually provides a pretty substantial benefit.

Update: Frozen string literals already provide what this PR is trying to accomplish. https://wyeworks.com/blog/2015/12/1/immutable-strings-in-ruby-2-dot-3

[parkr]

Yeah, double check this with ObjectSpace. In most ruby programs, the “frozen string literal” magic comment is all the optimization you need.

In my tests, jekyll is slow because of how long it takes to render using kramdown. It’s hands-down the Markdown parser that is making Jekyll slow — and doing most of the work. I’d like to explore bringing commonmark support into jekyll core to give folks an alternative (and a standard which reduces the learning curve if they already know how to use commonmark).

[parkr]

(The commonmark ruby gem uses the commonmark C library and it orders of magnitude faster than kramdown.)

[DirtyF]

👍 With jekyll-commonmark the build of Jekyll's website is 30% faster.

[pathawks]

We could require_with_graceful_fail("jekyll-commonmark") to add CommonMark as a converter, like we already do for redcarpet and rdiscount

[parkr]

@pathawks want to wire that up? I’ve been doing a little bit of testing with Pages (with an extension of your code in https://github.com/github/jekyll-commonmark-ghpages) and would love to wire it up in core so we don’t need to require anything but either commonmark or a jekyll plugin to get it working. Eventually (maybe 4.0?), i’d Love to see it as the default instead of kramdown.

[pathawks]

@parkr #6547

[parkr]

@pathawks Thanks! So I’d say this empty string optimization is unnecessary unless you see huge object savings in ObjectSpace.

[DirtyF]

Without any data to backup the supposed efficiency of this change, allow me to close this for now. Happy to re-open it if we see huge savings.

[DirtyF]

@parkr @pathawks https://github.com/mity/md4c is apparently the fastest C Markdown parser.