Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 410cdee6a40f
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: f4ede91ee098
Choose a head ref
  • 2 commits
  • 1 file changed
  • 2 contributors

Commits on Jun 5, 2015

  1. Don't call modify() on RubyStrings so eagerly when freezing them 

    This fixes #3019, where in certain cases it's possible to end up with
RubyString instances backed by very large unique ByteLists where only
a tiny portion of the bytes in the ByteList are actually needed.

What was happening is `modify` was being called on RubyString
instances too eagerly, resulting in unnecessary duplication of their
underlying ByteList instances instead of sharing when possible.

The two changes here are in `RubyString#newFrozen` and
`RubyString#resize` and I believe are more correct based on the C
implementation of these methods.

For `RubyString#newFrozen`, which roughly corresponds to
`rb_str_new_frozen` in
C (https://github.com/ruby/ruby/blob/v2_2_2/string.c#L932), the logic
looks to me to return the shared string and I don't see it calling
`str_make_independent`, which is roughly the same thing as
`RubyString#modify`. So, I just removed the call to `modify`.

For `RubyString#resize`, which roughly corresponds to to
`rb_str_resize` in C, shared strings are only made independent if
their length
differ (https://github.com/ruby/ruby/blob/v2_2_2/string.c#L2155). Thus,
instead of unconditionally calling `modify` here, I moved it into the
conditions that are true when the length differs.
    @bbrowning
    bbrowning committed Jun 5, 2015
    Copy the full SHA
    4fb81b7 View commit details

  2. Merge pull request #3022 from bbrowning/string-cow-oom 

    Don't call modify() on RubyStrings so eagerly when freezing them
    @enebo
    enebo committed Jun 5, 2015
    Copy the full SHA
    f4ede91 View commit details
Showing with 2 additions and 2 deletions.
  1. +2 −2 core/src/main/java/org/jruby/RubyString.java
4 changes: 2 additions & 2 deletions core/src/main/java/org/jruby/RubyString.java
View file
Original file line number Diff line number Diff line change
@@ -965,18 +965,18 @@ public RubyString newFrozen() {
klass = getMetaClass();
str = strDup(klass.getClassRuntime());
str.setCodeRange(getCodeRange());
str.modify();
str.setFrozen(true);
return str;
}

/** rb_str_resize
*/
public final void resize(int length) {
modify();
if (value.getRealSize() > length) {
modify();
value.setRealSize(length);
} else if (value.length() < length) {
modify();
value.length(length);
}
}