Details
-
Type:
Improvement
-
Status:
Open
-
Priority:
Major
-
Resolution: Unresolved
-
Affects Version/s: JRuby 1.6RC1
-
Fix Version/s: JRuby 1.6.4
-
Component/s: Core Classes/Modules
-
Labels:None
-
Number of attachments :
Description
This bug:
http://redmine.ruby-lang.org/issues/show/4298
Shows how getting the nth character from a string causes it to calculate the string's length each time. Ruby-core fixed this by not checking the length and modifying str_utf8_nth to rewrite the "beg" reference as seen in this commit:
https://github.com/ruby/ruby/commit/b0eaf0f621353b9075ff5ad8e4cd663da6a36dce
We should try to fix it in a similar way. My first attempt at a patch doesn't do the beg rewriting, but does improve the performance and eliminate the full-string scan each time:
diff --git a/src/org/jruby/RubyString.java b/src/org/jruby/RubyString.java
index 133fd7d..5b2149c 100644
--- a/src/org/jruby/RubyString.java
+++ b/src/org/jruby/RubyString.java
@@ -3286,10 +3286,11 @@ public class RubyString extends RubyObject implements EncodingCapable {
beg += strLength(enc);
if (beg < 0) return runtime.getNil();
}
- } else if (beg > 0 && beg > strLength(enc)) {
+ } else if (beg > 0 && beg > value.getRealSize()) {
return runtime.getNil();
}
if (len == 0) {
+ if (beg > strLength(enc)) return runtime.getNil();
p = 0;
} else if (isCodeRangeValid() && enc instanceof UTF8Encoding) {
p = StringSupport.utf8Nth(bytes, s, end, beg);
@@ -3298,8 +3299,7 @@ public class RubyString extends RubyObject implements EncodingCapable {
int w = enc.maxLength();
p = s + beg * w;
if (p > end) {
- p = end;
- len = 0;
+ return runtime.getNil();
} else if (len * w > end - p) {
len = end - p;
} else {
I'm not sure if this is a safe change or not.
Lopex and I were looking into ways of avoiding strLength calculation and I found one which could do it for 1/enc.maxLength of the cases. The above patch will probably only do it for a subset of index which are towards the end of the string.
Personally, I would like to know why strlength is not cached in the string and changed on modification. Virtually all modifications (small set like '*' which don't need to) end up recalculating code range which basically walks the whole string. Seems like a no-brainer for solving this problem since it would return both the code range and the new length.