Feature #19737: Add `IO::Buffer#cat` for concat `IO::Buffer` instances

• Status: Rejected
• Assignee: ioquatix (Samuel Williams)
• Target version: -

[unasuke (Yusuke Nakamura)]

motivation

In my use case, I want to concat two IO::Buffer instances. But current implementation doesn't have that way.
Then I created a patch. Opend here: TBD

concern

I have two concerns about it.

1. Should we provide IO::Buffer#+ as an alias?

In String instance, "a" + "b" returns "ab",. It feels intuitive.

So, should we provide the same way as IO::Buffer.for("a") + IO::Buffer.for("b")?
If + is provided, I naturally assume that * is also provided as an operator.
Should we also provide an IO::Buffer#* method for symmetry with the String class?

I thought the behavior of the "*" method is not obvious to me... (Is it right to just return joined buffers?)

2. Should it accept multiple IO::Buffer instances?

In the cat command, it accepts multiple inputs like this.

$ cat a.txt b.txt c.txt
a
b
c

Should IO::Buffer#cat accept multiple inputs too?

[unasuke (Yusuke Nakamura)]

Opend here https://github.com/ruby/ruby/pull/7960

[Eregon (Benoit Daloze)]

Could you include your actual concrete use case?
See https://github.com/ruby/ruby/wiki/How-To-Request-Features

Regarding naming, cat or concat feels wrong, + seems a much better match for the proposed semantics (no mutation).

I'm unsure if it's a good idea to concatenate IO::Buffer's, maybe it's not really meant for that, and it wouldn't be particularly fast.
Maybe it's easy to workaround by converting to String first for the probably-rare cases it's needed?

[ioquatix (Samuel Williams)]

Thanks for working on this.

So, in short, I'm against the name (even if it is cute), but I'm okay with considering the functionality.

I'd like to know more the use case.

We shouldn't encourage users to create lots of temporary buffers. It's better to construct one buffer and append into it.

The current functionality for "appending" to a buffer is copy.

a = IO::Buffer.for("A")
b = IO::Buffer.for("B")
c = IO::Buffer.new(2)
c.copy(a, 0)
c.copy(b, 1)
c
=> 
#<IO::Buffer 0x0000600001ec08f0+2 INTERNAL>
0x00000000  41 42                                           AB

The thing about buffers, is that reallocation and resizing is expensive, unlike Strings. This is by design.

It's a bad idea to write something like:

c = a + b
c.resize(1024)
c.copy(more_data, 2)

This is why I want to know the use case in a bit more detail.

[unasuke (Yusuke Nakamura)]

My use case is this: concat given two buffers, then modify (copy, slice, xor) it.
(More concretely, I want to apply/remove header protection in the QUIC protocol.)

I understood that using copy method is a better way than concat buffers.

But, described style is a bit redundant for me (especially size calc).

a = IO::Buffer.for(unknown_length_data)
b = IO::Buffer.for(from_somewhere)
c = IO::Buffer.new(a.size + b.size)
c.copy(a, 0)
c.copy(b, a.size)
pp c
# =>
# #<IO::Buffer 0x0000600003f021a0+2 INTERNAL>
# 0x00000000  41 42                                           AB

Is it API design? If so, I accept this style.

[mame (Yusuke Endoh)]

• Assignee set to ioquatix (Samuel Williams)

[ioquatix (Samuel Williams)]

• Status changed from Open to Rejected

It's probably not obvious given how existing Ruby code traditionally works, but in the above code, a and b are zero copy (which is ideal).

irb(main):001> data = "foo"
=> "foo"
irb(main):002> IO::Buffer.for(data)
=> 
#<IO::Buffer 0x00007fe0af7cd858+3 EXTERNAL READONLY SLICE>
0x00000000  66 6f 6f                                        foo

Note, it says "EXTERNAL READONLY SLICE". This means, the buffer is referring to external memory (the original string). Therefore your proposed solution, only does the bare minimum memory copies.

I believe this is good enough to start with. So, I want to reject your proposal for now.

However, it doesn't mean we shouldn't reconsider it for the future. So, if you find additional use cases, please come and discuss it :)

Thanks for your effort and contribution.