Bytes are still fine

[glyph]

fixes #195

[Kriechi]

Thanks - I'm fine with the changes. Please add a changelog entry to document the reversal of the most recent minor API change back to what it was two versions before.

Just for thinking it through: would the downstream Twisted benefit in any way from adopting bytearrays (instead of sticking with the "old" bytes-only API)? My assumption is that bytearrays are more performant - though I don't have data to back up this claim. I overlooked that the last change actually caused API breakage, because I misread the other ticket in a way that indicated it already was using bytesarray but not using the appropriate type hints.

[glyph]

Just for thinking it through: would the downstream Twisted benefit in any way from adopting bytearrays (instead of sticking with the "old" bytes-only API)?

I can't imagine how. We are already receiving bytes from socket.recv(), so converting to bytearray would be entirely an imposition of overhead, having to call the bytearray constructor and then not leveraging any of its advantages.

My assumption is that bytearrays are more performant - though I don't have data to back up this claim.

It would be good to set up codspeed to have a tool that can quickly validate questions like this.

For example, I would assume that the change I'm making here is a slight optimization, because it avoids calling a constructor and making a copy. "more performant" as a context-free term doesn't make much sense. More performant at what? bytearray objects have a big advantage over bytes in that they are mutable and thus can be used via, e.g., socket.recv_into, which could avoid an allocation. But then you need to be scrupulously careful not to give up those gains by making any copies during parsing of data in that buffer, which is really tricky.

I overlooked that the last change actually caused API breakage, because I misread the other ticket in a way that indicated it already was using bytesarray but not using the appropriate type hints.

We really ought to have pins in our CI and dedicated downstream tests, so not exactly your fault for breaking a downstream (especially a new one).

But, it would probably be good moving forward to have some tests for actually using bytearrays to make sure we're doing the right thing.

[glyph]

@Kriechi changelog entry added