re-implement (native) IOBuffer for JRuby #1691
Conversation
... to match up JRuby's ext with the C ext
|
@headius Would you mind giving this a review? |
|
@enebo Would you be able to give this a review? 🔍 👁🗨 🔎 |
|
Oops, never saw this. @enebo or I would be fine, but I'm traveling now.
…On Mon, Jan 21, 2019, 01:00 Olle Jonsson ***@***.*** wrote:
@enebo <https://github.com/enebo> Would *you* be able to give this a
review?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1691 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAAnl0dBS4Evc2LOjRQwiBlgecv3eKOoks5vFMPKgaJpZM4ZcCAV>
.
|
|
|
||
| @JRubyMethod | ||
| public IRubyObject reset() { | ||
| buffer.setRealSize(0); |
There was a problem hiding this comment.
Not sure this is actionable but this won't reduce/remove backing array of bytelist. I would not worry about it but if someone loads a massive IOBuffer and it is reused it will just drag around a massive backing array as long as it lives.
There was a problem hiding this comment.
which is exactly the same behaviour as puma has using ByteArrayOutputStream#reset ... thus such a debate would be for later (ideally reported in an issue if you feel its really a problem).
There was a problem hiding this comment.
@kares yeah I agree and did notice it was like this before as well.
|
@kares I did approve the review but I can explicitly state it here as well. This looks good to be merged. |
|
sorry about that I removed the comment - was half asleep and did not notice the approval first. |
|
@kares no worries. I agree my comment would be confusing to read even with the approval. I should have probably mentioned it after the merge. |
|
@kares @olleolleolle I know this was motivated by Java 9+ warnings but do you anticipate any perf gains now that this landed? |
|
not really, except for some byte copy-ing ... maybe slightly less mem use (not sure what the buff is used for) |
... to match up JRuby's ext with the C ext
extensions now provide the same functionality - no need for branching (around
Puma::IOBuffer)this has been motivated by warnings on Java 9+ due
field_reader :bufhaving protected accesssince JRuby does not generate native classes for Java sub-classes (atm): jruby/jruby#5532