Skip to content

Enforce Style/FrozenStringLiteralComment#6265

Merged
jekyllbot merged 1 commit into
masterfrom
frozen-string-literal
Aug 4, 2017
Merged

Enforce Style/FrozenStringLiteralComment#6265
jekyllbot merged 1 commit into
masterfrom
frozen-string-literal

Conversation

@parkr

@parkr parkr commented Aug 3, 2017

Copy link
Copy Markdown
Member

The frozen_string_literal: true magic comment in Ruby can help
dramatically decrease memory allocations for new strings and can thusly
speed up your program. The intent here is for Jekyll to use less memory
and make fewer memory allocations (which must later be GC'd).

Using the output of GC.stats from script/stackprof:

Before:

total_allocated_pages: 67284
total_allocated_objects: 27434203
total_freed_objects: 23567

After:

total_allocated_pages: 63008
total_allocated_objects: 25690804
total_freed_objects: 22907

With this patch, we allocate 4,276 fewer pages, and 1,743,399 fewer objects. Holy smokes! That's huge savings.

/cc @jekyll/stability

@jekyllbot jekyllbot assigned parkr and ghost Aug 3, 2017
@parkr parkr requested review from mattr- and pathawks August 3, 2017 23:31
@parkr parkr force-pushed the frozen-string-literal branch from ba4e048 to e788ad5 Compare August 3, 2017 23:37

@pathawks pathawks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A couple comments, but I'm totally on board with this change 👍

Comment thread docs/_docs/history.md Outdated
- Update minimum Ruby version in installation.md ([#6164]({{ site.repository }}/issues/6164))
- [docs] Add information about finding a collection in `site.collections` ([#6165]({{ site.repository }}/issues/6165))
- Add {%raw%} to Liquid example on site ([#6179]({{ site.repository }}/issues/6179))
- Add {% raw %}`{% raw %}`{% endraw %} to Liquid example on site ([#6179]({{ site.repository }}/issues/6179))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This probably doesn't belong in this PR
?

Comment thread exe/jekyll Outdated
@@ -1,4 +1,6 @@
#!/usr/bin/env ruby
# frozen_string_literal: true
#

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should probably be an empty line

Comment thread lib/jekyll/mime.types Outdated
text/x-java-source java
text/x-lua lua
text/x-markdown markdown md mkd
text/x-markdown mkd

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Mime type stuff probably doesn't belong in this PR
?

The frozen_string_literal: true magic comment in Ruby can help
dramatically decrease memory allocations for new strings and can thusly
speed up your program. The intent here is for Jekyll to use less memory
and make fewer memory allocations (which must later be GC'd).
@parkr parkr force-pushed the frozen-string-literal branch from e788ad5 to 4d1659c Compare August 4, 2017 01:05
@parkr

parkr commented Aug 4, 2017

Copy link
Copy Markdown
Member Author

Updated, thanks for the review @pathawks!

@jekyllbot: merge +fix

@jekyllbot jekyllbot merged commit 7cf5f51 into master Aug 4, 2017
@jekyllbot jekyllbot deleted the frozen-string-literal branch August 4, 2017 01:27
jekyllbot added a commit that referenced this pull request Aug 4, 2017
@jekyll jekyll locked and limited conversation to collaborators Jan 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants