Skip to content

Enforce Style/FrozenStringLiteralComment#6265

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

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

Conversation

@parkr
Copy link
Member

@parkr parkr commented Aug 3, 2017

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
Copy link
Member

@pathawks pathawks left a comment

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 👍

- 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
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
?

exe/jekyll Outdated
@@ -1,4 +1,6 @@
#!/usr/bin/env ruby
# frozen_string_literal: true
#
Copy link
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

text/x-java-source java
text/x-lua lua
text/x-markdown markdown md mkd
text/x-markdown mkd
Copy link
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
Copy link
Member Author

parkr commented Aug 4, 2017

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