Feature #889: erb.rb should use Array and << for eoutvar and not String and concat

• Status: Rejected
• Assignee: -

[enebo (Thomas Enebo)]

=begin
Repeated String concatenation is not as fast or efficient as an array.join('') on a list of strings since the array can know at join time how big the result string should be.

This makes a decent perf difference and is quite a bit gentler on memory allocation.

I attached a patch. This appears to be applicable on all versions of Ruby.
=end

[nex3 (Natalie Weizenbaum)]

=begin
This will cause all sorts of incompatibilities with at least Rails, which expects ERB to be building up a string that it can slice and so forth.
=end

[enebo (Thomas Enebo)]

=begin
By the end of the generated template it will return a string. So anything coming out is a string which can be sliced etc...Or are you saying it needs to be a string in the middle of building a template?
=end

[nex3 (Natalie Weizenbaum)]

=begin
Yeah, Rails has helpers that slice up the string and generally assume that it's going to be a string at every point during rendering.
=end

[headius (Charles Nutter)]

=begin
Slicing the string in mid-render seems like a really bad idea, since because of copy-on-write this will cause any appends after slicing to have to make a full new copy of the string contents. Perhaps Rails should be modified to not require this behavior, since the modified template rendering is substantially faster.
=end

[bitsweat (Jeremy Daer)]

=begin
Rails 2.2 and later use buffer swapping, not string slicing, to render blocks in ERB. Nowhere else does Rails assume the buffer is a string.

Earlier Rails are already incompatible with the latest Enumerable changes in 1.8.7, so this change would not break compatibility for anyone.
=end

[shyouhei (Shyouhei Urabe)]

• Assignee set to seki (Masatoshi Seki)

=begin

=end

[mame (Yusuke Endoh)]

• Status changed from Open to Feedback
• Assignee deleted (seki (Masatoshi Seki))

=begin
Hi,

Repeated String concatenation is not as fast or efficient as an array.join('') on a list of strings since the array can know at join time how big the result string should be.

Similar idea was said in [ruby-core:28493]. I like this idea,
but I cannot observe performance improvement by Array#join:

$ ./ruby -v
ruby 1.9.2dev (2010-03-25 trunk 27045) [i686-linux]

original

$ time ./ruby -Ilib benchmark/bm_app_erb.rb

real 0m1.876s
user 0m1.872s
sys 0m0.004s

use "<<" instead of ".concat"

$ time ./ruby -Ilib benchmark/bm_app_erb.rb

real 0m1.813s
user 0m1.804s
sys 0m0.016s

with proposed patch

$ time ./ruby -Ilib benchmark/bm_app_erb.rb

real 0m1.824s
user 0m1.820s
sys 0m0.004s

This seems to show using "<<" makes a difference, instead of
Array#join. I guess it reduces code size emitted by ERB.
But I could be wrong because I just skimped a benchmark.

Could you give me a benchmark proving that Array#join brings
us performance improvement?

--
Yusuke Endoh [e-mail]
=end

[coatl (caleb clausen)]

=begin
Yusuke, as I read it, your numbers show a very slight improvement, (perhaps not statistically significant?) for the proposed change (and even slighter degradation versus a variant of the status quo). My recollection from the discussion of the other bug you cite, #905, was that the looked-for performance gain was not there because (at least in part) the implementation of realloc on linux is particularly smart, making use of memory mapping to avoid copying large buffers.

However, other systems (notably windows, perhaps macos?) might not have such a good realloc available to them. If so, there might be a larger improvement to be seen on those platforms, which could make this change (and #905) worthwhile just for the sake of those platforms.

Does someone using macos or (especially) windows want to post some performance numbers for this patch?
=end

[mame (Yusuke Endoh)]

=begin
Hi,

Yusuke, as I read it, your numbers show a very slight improvement, (perhaps not statistically significant?) for the proposed change (and even slighter degradation versus a variant of the status quo).

Agreed. Though I believe it is actually improved (I confirmed by
running each configuration ten times), the improvement is subtle.

My recollection from the discussion of the other bug you cite, #905, was that the looked-for performance gain was not there because (at least in part) the implementation of realloc on linux is particularly smart, making use of memory mapping to avoid copying large buffers.

It is true when ERB generates string as big as page size (4KB in
many platform). But benchmark/bm_app_erb.rb generates just 247
bytes in each iteration.
So there may be a chance to improve it against the benchmark.

--
Yusuke Endoh [e-mail]

=end

[kstephens (Kurt Stephens)]

=begin
This is not a good idea, because the expression value Strings accumulated in the Array must be protected from mutation.
ERB expressions can have side-effects.

 SOME_STRING = 'foobar'
 def foo
   SOME_STRING
 end
 def bar
   SOME_STRING.sub!(/bar/, '')
   SOME_STRING
 end
 # ERB GENERATED CODE: from "<%= foo %><%= bar %>"
 eoutvar = [ ]
 eoutvar << foo
 eoutvar << bar
 eoutvar.join('')
 

=end

[bitsweat (Jeremy Daer)]

=begin
On Fri, Mar 26, 2010 at 11:45 AM, Kurt Stephens [e-mail] wrote:

Issue #889 has been updated by Kurt  Stephens.

This is not a good idea, because the expression value Strings accumulated in the Array must be protected from mutation.
ERB expressions can have side-effects.

SOME_STRING = 'foobar'
def foo
 SOME_STRING
end
def bar
 SOME_STRING.sub!(/bar/, '')
 SOME_STRING
end
# ERB GENERATED CODE: from "<%= foo %><%= bar %>"
eoutvar = [ ]
eoutvar << foo
eoutvar << bar
eoutvar.join('')

I disagree. That's based on a loose assumption that <%= ... %> behaves
like #{...}.

But all it says is "output" not necessarily "interpolate" -- that's a
side effect of the current implementation. Much like using <% .... #
comment %> was an assumption that later broke.

jeremy

=end

[kstephens (Kurt Stephens)]

=begin
String#<<(x) "appends by value", Array#<<(x) "appends by reference". The subsequent Array#join is affected by mutations to the argument, the former is not. Ruby Strings are passed and returned by reference, not by value.

The proposal introduces aliasing problems that did not exist before.

IMO, it's counter-intuitive. <%= ... %> currently behaves as if it's embedded in a stream, similarly "#{...}" interpolations are evaluated left-to-right. String#<< and IO#<< have stream-like semantics and are immune to argument aliasing, Array#<< and the subsequent Array#join behave differently.

The example above would work differently, if IO === eoutvar, than if Array === eoutvar. If we don't care about preserving semantics, then it's not an issue.

I'd prefer that <%= ... %> continues to behave like "#{...}", both
have implicit order-of-evaluation and are immune to aliasing.

=end

[coatl (caleb clausen)]

=begin
I'm afraid that I have to agree with Kurt, in that the changed behavior of this proposal would cause too many problems for existing users. Thanks for spotting that, Kurt.

Instead maybe #dup the string before adding it to erb's buffer? This might actually perform somewhat reasonably since ruby seems to create a copy-on-write string when you #dup. But it's likely to erase any small gain that had been achieved by avoiding extra reallocs.

Another possibility might be to have erb write directly to the output file (or in this case, socket). I was excited about this possibility, but on reflection it seems fraught with problems.
=end

[mame (Yusuke Endoh)]

• Status changed from Feedback to Rejected

=begin
Hi,

2010/3/27 Kurt Stephens [e-mail]:

This is not a good idea, because the expression value Strings accumulated in the Array must be protected from mutation.
ERB expressions can have side-effects.

That's a good point. I never thought of that. Thank you!

This proposal not only brings little performance gain, but also leads
to incompatibility.

I reject the ticket. If you come up with another idea that solves
the two problems, please register a new ticket with benchmark.

Thanks,

--
Yusuke ENDOH [e-mail]
=end