RFR: 8330532: Improve line-oriented text parsing in HotSpot [v3]

[Ioi Lam]

On Wed, 24 Apr 2024 09:43:17 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> The `override` keyword is nice; thank you.
>> 
>> I have already argued against the removal of `set_input`.  And `set_input` needs `close`.
>> 
>> I think `set_input` is not YAGNI but YIWNI = Yes I will need it.  The reply that “you can just wrap another i-stream around the new i-source” is fallacious because of the performance model of i-stream.
>
> Sorry, I'm still not on board with the `close` operation and I'm against `set_input` calling `close()` :-). Why is it necessary for the `inputStream` to require a file to be re-opened if the `inputStream` switches from one file to another?
> 
> To be clear: OK, we want `set_input` because we don't want to allocate two small buffers, that's fine by me.

@jdksjolen I have incorporated most of your suggested changes. I left this code in for now:


void inputStream::set_input(inputStream::Input* input) {
  clear_buffer();
  if (_input != nullptr && _input != input) {
    _input->close();
  }
  ...


I am also leaning towards removing the `close()` call. Otherwise it would be unsymmetrical - the `inputStream` doesn't open the `_input` automatically, but it will close it automatically for us.

It seems better to leave both the `open` and `close` to the caller the `inputStream`.

@rose00 what do you think?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18833#discussion_r1586789727