Hi Daniel,
CSR's get few reviewers than PR's.
A broader review in the PR can improve the prose and get comments on
additional use cases.
For myself, I do them in parallel, putting the initial CSR in proposed
state to get early feedback from the CSR review.
Comments on the PR accumulate and are addressed in the PR and when that
settles down, update the PR and finalize.
In the CSR, the Compatibility Risk description highlights a typical more
serious risk, that of superseding an existing method in an
implementation and possibly changing or being in conflict with its
semantics. As a protected method, its potential to be in conflict with
existing implementations is reduced somewhat.
The diff in the CSR should focus on the specification change, not the
implementation changes unless they affect visible behavior. (Generally,
avoid irrelevant information in the CSR).
BTW, P4 is a better priority for this useful addition, P5 is pretty much
ignored or trivial and not worth it.
Thanks for the followup, Roger
On 1/7/26 4:07 PM, Daniel Gredler wrote:
Hi Roger,
I've created the CSR here: https://bugs.openjdk.org/browse/JDK-8374739
Alan also provided some feedback on the original issue here:
https://bugs.openjdk.org/browse/JDK-8374610
I was going to wait to create the PR until the CSR has been approved,
if that's OK?
Take care,
Daniel
On Tue, Jan 6, 2026 at 9:05 PM Daniel Gredler <[email protected]> wrote:
> It would risky to change the implementation of
BAOS.ensureCapacity to not throw OOME on negative minCapacity.
Completely agree -- if we wanted the public API contract to match
the `ensureCapacity` method behavior elsewhere, we would leave the
existing method untouched and private, and add a public version
which only delegates to the existing private method if minCapacity
> 0 (almost identical to `AbstractStringBuilder`).
Take care,
Daniel
On Tue, Jan 6, 2026 at 8:53 PM Roger Riggs
<[email protected]> wrote:
Hi Daniel,
I stand corrected, any current size (>=0) satisfies the
request (zero is greater than -1).
I would not say it "ignores" it, just that it already is
greater or equal to the request.
The current phrasing of ByteArrayOutputStream.ensureCapacity
uses the "holds at least" phrasing.
It would risky to change the implementation of
BAOS.ensureCapacity to not throw OOME on negative minCapacity.
Roger
On 1/6/26 1:30 PM, Daniel Gredler wrote:
Hi Roger,
All points noted, thanks -- just wanted to double check on this:
> I don't think you could call it `ensureCapacity()` if it
ignores the request.
If you pass a negative (i.e. invalid) argument, you need to
either throw an exception or ignore the request. Both
`ArrayList` and `StringBuilder` simply ignore calls with
negative arguments (i.e. neither `new
ArrayList().ensureCapacity(-1)` nor `new
StringBuilder().ensureCapacity(-1)` throw an exception, they
just ignore the request). The old `Vector` class does the
same, though few developers will be using it at this stage.
All of that to say that as a developer, ignoring an
`ensureCapacity()` call with a negative capacity would not
have surprised me in the least, and would be consistent with
the other `ensureCapacity` methods in other core classes.
Let me know your thoughts on this final point, and I'll move
forward with both the CSR and the PR.
Thanks!
Daniel
On Tue, Jan 6, 2026 at 3:54 PM Roger Riggs
<[email protected]> wrote:
Hi Daniel,
I don't think you could call it `ensureCapacity()` if it
ignores the request.
The limit is not specified, different VMs have different
overheads.
The exception and message are appropriate:
"java.lang.OutOfMemoryError: Requested array size exceeds
VM limit "
For negative arguments, an IllegalArgumentException might
be an improvement in usability for developers.
[2] 8258565 is closed as "will not fix" because the
memory limits are an implementation parameter, not specified.
Thanks, Roger
p.s. as an API change it will need a CSR. There's a
"create a CSR" menu item on the issue and a template to
fill out.
On 1/6/26 9:01 AM, Daniel Gredler wrote:
Hi Roger,
Thanks for the feedback, I've created an issue in JBS
[1] and will create the PR in the coming days.
This method currently throws an OOME if a negative
capacity is requested, but that should never happen
because it's only called internally after careful
parameter validation. Once it's public, users will be
able to request negative capacities directly. For the
public API, I'm leaning towards quietly ignoring calls
with non-positive capacities, a la
`AbstractStringBuilder`, instead of throwing an OOME.
Let me know if you agree?
Also, do you know what the maximum possible requested
capacity is here? It would be good to avoid the need for
follow-up issues like this one [2] for `ArrayList`.
Take care,
Daniel
[1] https://bugs.openjdk.org/browse/JDK-8374610
[2] https://bugs.openjdk.org/browse/JDK-8258565
On Mon, Jan 5, 2026 at 3:54 PM Roger Riggs
<[email protected]> wrote:
Hi Daniel,
That seems reasonable, allowing control over the
timing of resizes.
Making it public is sensible too.
From a higher point of view, are you sure you want
to be working with
ByteArrayOutputStream and not ByteBuffer or Memory
Segments? There are
more performance possibilities with those APIs.
Regards, Roger
On 1/5/26 7:15 AM, Daniel Gredler wrote:
> Hi,
>
> I was recently looking at subclassing
`ByteArrayOutputStream` in an
> application so that I could add a fast
VarHandle-based
> `writeLong(long)` method (writing 8 bytes to the
byte array in one
> go). The internal `ByteArrayOutputStream` buffer
is protected, so no
> issue there, but `ensureCapacity(int)` is private
rather than
> protected, and uses the internal `ArraysSupport`
class, so it's not
> even easy to copy/paste as duplicate code in the
subclass. Similar
> `ensureCapacity` methods in `ArrayList` and
`StringBuilder` are
> public. Would a PR adjusting the visibility of
this method from
> private to protected be accepted?
>
> Take care,
>
> Daniel
>
>
