Re: 8252827: Caching Integer.toString just like Integer.valueOf

[Maurizio Cimadamore]

Ciao Raffaello,
another general consideration: caching sometimes makes certain 
optimizations harder to pull off.


Few years ago I've seen this talk at Fosdem:

https://archive.fosdem.org/2020/schedule/event/reducing_gc_times/

See minute 5:53 - where it shows that cached Integer values (as per 
Integer::valueOf) inhibit scalarization, and using `new Integer(...)` 
leads to superior performances.


It's a pattern I have observed several time when working with the 
Foreign Memory API, where most of the abstractions are immutable: 
attempting to cache things (e.g. MemoryAddress instances) often comes up 
with the same GC overhead (because these instances are escape analyzed 
away) and ends up with worse performance overall (loss of scalarization).


Of course this is not _always_ the case - but I wanted to add this 
particular angle to the discussion.


Cheers
Maurizio

On 16/04/2021 17:48, Raffaello Giulietti wrote:

Hello,

does the enhancement proposed in [1] make sense, both today and when 
wrappers will be migrated to primitive classes?

If so, it should be rather simple to add it and I could prepare a PR.
If not, the issue might perhaps be closed.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8252827

Re: 8252827: Caching Integer.toString just like Integer.valueOf

[Stuart Marks]

(Catching up on old email threads.)

I don't have much to add to the technical discussion here. Yes, caching the Integer 
instances seems obsolescent, and it seems unlikely that caching String conversions 
will be helpful. I've gone ahead and closed out the bug. [1]


On triaging bugs... we do triage bugs pretty effectively, I think. However, 
enhancement requests (like this one) tend to languish. People tend to notice ones 
that are obviously bad ideas or obviously good ideas and act on them quickly. 
However, there are a lot of items that are neither obviously good nor bad, so they 
just sit there. Sometimes they just sit there even after there has been some email 
discussion on them. :-)


Sometimes I think we take these requests a bit too seriously. It looks to me like 
the submitter put about five minutes of thought into the request, and we've 
collectively probably spent 10x that dealing with it. I probably put too much effort 
into this bug myself; instead of researching the history, I could have simply closed 
it with a comment saying, "Closed, per discussion in " which 
would have been reasonable.


Anyway, it's closed.

s'marks


[1] https://bugs.openjdk.java.net/browse/JDK-8252827


On 4/17/21 5:19 AM, Raffaello Giulietti wrote:

Hi,

in view of Integer becoming a primitive class [1], the IntegerCache is probably 
going to disappear.


For a small, fixed range like the one you are proposing [-1, 16], there's no real 
need for a separate cache class. You could have a switch in the implementation of 
toString(), with the string literals then being part of the constant pool of the 
class. Not free beer, but supported by the VM since day 0.


It's only when the range is open that you'd need a cache similar to 
IntegerCache.


My 2 cents as well :-)
Raffaello



[1] https://openjdk.java.net/jeps/402

On 2021-04-17 11:18, Laurent Bourgès wrote:

Hi,

I read the JBS bug and I interpret it as:
- IntegerCache provides Integer instances for [-128, 127] by default
- Having Integer.toString(int) could behave the same or at least cache most 
probable values like [-1 to 16] or using the IntegerCache range.


It looks trivial and potentially could benefits to jdk itself to reduce memory 
garbage : is Integer.toString(int) widely used in the jdk codebase ?


Finally it can be alternatively implemented in application's code.

My 2 cents,
Laurent

Le sam. 17 avr. 2021 à 11:06, Raffaello Giulietti > a écrit :




    On 2021-04-17 07:07, David Holmes wrote:
 > On 17/04/2021 4:54 am, Raffaello Giulietti wrote:
 >> I guess the reporter meant to limit the cache range similarly to
    the
 >> one used for valueOf().
 >>
 >> I have no clue about the benefit/cost ratio for the proposed String
 >> cache. It really depends on usage, workload, etc. One can easily
 >> imagine both extreme scenarios but it's hard to tell how the
    average
 >> one would look.
 >>
 >> My post is only about either solving the issue by implementing the
 >> cache, which I can contribute to; or closing it because of lack of
 >> real-world need or interest.
 >
 > Caching for the sake of caching is not an objective in itself.
    Unless
 > the caching can be shown to solve a real problem, and the
    strategy for
 > managing the cache is well-defined, then I would just close the
 > enhancement request. (Historically whether an issue we don't have
    any
 > firm plans to address is just left open "forever" or closed, depends
 > very much on who does the bug triaging in that area. :) )
 >
 > Cheers,
 > David
 >


    Indeed, the impression is that many of the issues are probably open
    because it's unclear whether they should be addressed with some
    implementation or spec effort or whether they should be closed.
    Triaging
    is certainly a harder job than it appears at first sight ;-)

    It would be useful to have a kind of "suspended" or "limbo" resolution
    state on the JBS for issues like this one, so people searching for more
    compelling open ones would not encounter them.

    Personally, I would close this issue without a "fix"; or "suspend" it.


    Greetings
    Raffaello



 >>
 >> Greetings
 >> Raffaello
 >>
 >>
 >> On 2021-04-16 20:36, Roger Riggs wrote:
 >>> Hi,
 >>>
 >>> Is there any way to quantify the savings?
 >>> And what technique can be applied to limit the size of the cache.
 >>> The size of the small integer cache is somewhat arbitrary.
 >>>
 >>> Regards, Roger
 >>>
 >>> On 4/16/21 12:48 PM, Raffaello Giulietti wrote:
  Hello,
 
  does the enhancement proposed in [1] make sense, both today
    and when
  wrappers will be migrated to primitive classes?
  If so, it should be rather simple to add it and I could
    prepare a PR.
  If not, the issue might perhaps be closed.
  

Re: 8252827: Caching Integer.toString just like Integer.valueOf

[Tagir Valeev]

Hello!

I would vote to close this. The benefit for all JDK users is questionable.
If it's necessary in a particular application, it could be implemented in
user code.

With best regards,
Tagir Valeev.

пт, 16 апр. 2021 г., 23:49 Raffaello Giulietti <
raffaello.giulie...@gmail.com>:

> Hello,
>
> does the enhancement proposed in [1] make sense, both today and when
> wrappers will be migrated to primitive classes?
> If so, it should be rather simple to add it and I could prepare a PR.
> If not, the issue might perhaps be closed.
>
>
> Greetings
> Raffaello
>
> 
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8252827
>

Re: 8252827: Caching Integer.toString just like Integer.valueOf

[Raffaello Giulietti]

Hi,

in view of Integer becoming a primitive class [1], the IntegerCache is 
probably going to disappear.


For a small, fixed range like the one you are proposing [-1, 16], 
there's no real need for a separate cache class. You could have a switch 
in the implementation of toString(), with the string literals then being 
part of the constant pool of the class. Not free beer, but supported by 
the VM since day 0.


It's only when the range is open that you'd need a cache similar to 
IntegerCache.



My 2 cents as well :-)
Raffaello



[1] https://openjdk.java.net/jeps/402

On 2021-04-17 11:18, Laurent Bourgès wrote:

Hi,

I read the JBS bug and I interpret it as:
- IntegerCache provides Integer instances for [-128, 127] by default
- Having Integer.toString(int) could behave the same or at least cache 
most probable values like [-1 to 16] or using the IntegerCache range.


It looks trivial and potentially could benefits to jdk itself to reduce 
memory garbage : is Integer.toString(int) widely used in the jdk codebase ?


Finally it can be alternatively implemented in application's code.

My 2 cents,
Laurent

Le sam. 17 avr. 2021 à 11:06, Raffaello Giulietti 
mailto:raffaello.giulie...@gmail.com>> a 
écrit :




On 2021-04-17 07:07, David Holmes wrote:
 > On 17/04/2021 4:54 am, Raffaello Giulietti wrote:
 >> I guess the reporter meant to limit the cache range similarly to
the
 >> one used for valueOf().
 >>
 >> I have no clue about the benefit/cost ratio for the proposed String
 >> cache. It really depends on usage, workload, etc. One can easily
 >> imagine both extreme scenarios but it's hard to tell how the
average
 >> one would look.
 >>
 >> My post is only about either solving the issue by implementing the
 >> cache, which I can contribute to; or closing it because of lack of
 >> real-world need or interest.
 >
 > Caching for the sake of caching is not an objective in itself.
Unless
 > the caching can be shown to solve a real problem, and the
strategy for
 > managing the cache is well-defined, then I would just close the
 > enhancement request. (Historically whether an issue we don't have
any
 > firm plans to address is just left open "forever" or closed, depends
 > very much on who does the bug triaging in that area. :) )
 >
 > Cheers,
 > David
 >


Indeed, the impression is that many of the issues are probably open
because it's unclear whether they should be addressed with some
implementation or spec effort or whether they should be closed.
Triaging
is certainly a harder job than it appears at first sight ;-)

It would be useful to have a kind of "suspended" or "limbo" resolution
state on the JBS for issues like this one, so people searching for more
compelling open ones would not encounter them.

Personally, I would close this issue without a "fix"; or "suspend" it.


Greetings
Raffaello



 >>
 >> Greetings
 >> Raffaello
 >>
 >>
 >> On 2021-04-16 20:36, Roger Riggs wrote:
 >>> Hi,
 >>>
 >>> Is there any way to quantify the savings?
 >>> And what technique can be applied to limit the size of the cache.
 >>> The size of the small integer cache is somewhat arbitrary.
 >>>
 >>> Regards, Roger
 >>>
 >>> On 4/16/21 12:48 PM, Raffaello Giulietti wrote:
  Hello,
 
  does the enhancement proposed in [1] make sense, both today
and when
  wrappers will be migrated to primitive classes?
  If so, it should be rather simple to add it and I could
prepare a PR.
  If not, the issue might perhaps be closed.
 
 
  Greetings
  Raffaello
 
  
 
  [1] https://bugs.openjdk.java.net/browse/JDK-8252827

 >>>

Re: 8252827: Caching Integer.toString just like Integer.valueOf

[Laurent Bourgès]

Hi,

I read the JBS bug and I interpret it as:
- IntegerCache provides Integer instances for [-128, 127] by default
- Having Integer.toString(int) could behave the same or at least cache most
probable values like [-1 to 16] or using the IntegerCache range.

It looks trivial and potentially could benefits to jdk itself to reduce
memory garbage : is Integer.toString(int) widely used in the jdk codebase ?

Finally it can be alternatively implemented in application's code.

My 2 cents,
Laurent

Le sam. 17 avr. 2021 à 11:06, Raffaello Giulietti <
raffaello.giulie...@gmail.com> a écrit :

>
>
> On 2021-04-17 07:07, David Holmes wrote:
> > On 17/04/2021 4:54 am, Raffaello Giulietti wrote:
> >> I guess the reporter meant to limit the cache range similarly to the
> >> one used for valueOf().
> >>
> >> I have no clue about the benefit/cost ratio for the proposed String
> >> cache. It really depends on usage, workload, etc. One can easily
> >> imagine both extreme scenarios but it's hard to tell how the average
> >> one would look.
> >>
> >> My post is only about either solving the issue by implementing the
> >> cache, which I can contribute to; or closing it because of lack of
> >> real-world need or interest.
> >
> > Caching for the sake of caching is not an objective in itself. Unless
> > the caching can be shown to solve a real problem, and the strategy for
> > managing the cache is well-defined, then I would just close the
> > enhancement request. (Historically whether an issue we don't have any
> > firm plans to address is just left open "forever" or closed, depends
> > very much on who does the bug triaging in that area. :) )
> >
> > Cheers,
> > David
> >
>
>
> Indeed, the impression is that many of the issues are probably open
> because it's unclear whether they should be addressed with some
> implementation or spec effort or whether they should be closed. Triaging
> is certainly a harder job than it appears at first sight ;-)
>
> It would be useful to have a kind of "suspended" or "limbo" resolution
> state on the JBS for issues like this one, so people searching for more
> compelling open ones would not encounter them.
>
> Personally, I would close this issue without a "fix"; or "suspend" it.
>
>
> Greetings
> Raffaello
>
>
>
> >>
> >> Greetings
> >> Raffaello
> >>
> >>
> >> On 2021-04-16 20:36, Roger Riggs wrote:
> >>> Hi,
> >>>
> >>> Is there any way to quantify the savings?
> >>> And what technique can be applied to limit the size of the cache.
> >>> The size of the small integer cache is somewhat arbitrary.
> >>>
> >>> Regards, Roger
> >>>
> >>> On 4/16/21 12:48 PM, Raffaello Giulietti wrote:
>  Hello,
> 
>  does the enhancement proposed in [1] make sense, both today and when
>  wrappers will be migrated to primitive classes?
>  If so, it should be rather simple to add it and I could prepare a PR.
>  If not, the issue might perhaps be closed.
> 
> 
>  Greetings
>  Raffaello
> 
>  
> 
>  [1] https://bugs.openjdk.java.net/browse/JDK-8252827
> >>>
>

Re: 8252827: Caching Integer.toString just like Integer.valueOf

[Raffaello Giulietti]

On 2021-04-17 07:07, David Holmes wrote:

On 17/04/2021 4:54 am, Raffaello Giulietti wrote:
I guess the reporter meant to limit the cache range similarly to the 
one used for valueOf().


I have no clue about the benefit/cost ratio for the proposed String 
cache. It really depends on usage, workload, etc. One can easily 
imagine both extreme scenarios but it's hard to tell how the average 
one would look.


My post is only about either solving the issue by implementing the 
cache, which I can contribute to; or closing it because of lack of 
real-world need or interest.


Caching for the sake of caching is not an objective in itself. Unless 
the caching can be shown to solve a real problem, and the strategy for 
managing the cache is well-defined, then I would just close the 
enhancement request. (Historically whether an issue we don't have any 
firm plans to address is just left open "forever" or closed, depends 
very much on who does the bug triaging in that area. :) )


Cheers,
David




Indeed, the impression is that many of the issues are probably open 
because it's unclear whether they should be addressed with some 
implementation or spec effort or whether they should be closed. Triaging 
is certainly a harder job than it appears at first sight ;-)


It would be useful to have a kind of "suspended" or "limbo" resolution 
state on the JBS for issues like this one, so people searching for more 
compelling open ones would not encounter them.


Personally, I would close this issue without a "fix"; or "suspend" it.


Greetings
Raffaello





Greetings
Raffaello


On 2021-04-16 20:36, Roger Riggs wrote:

Hi,

Is there any way to quantify the savings?
And what technique can be applied to limit the size of the cache.
The size of the small integer cache is somewhat arbitrary.

Regards, Roger

On 4/16/21 12:48 PM, Raffaello Giulietti wrote:

Hello,

does the enhancement proposed in [1] make sense, both today and when 
wrappers will be migrated to primitive classes?

If so, it should be rather simple to add it and I could prepare a PR.
If not, the issue might perhaps be closed.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8252827

Re: 8252827: Caching Integer.toString just like Integer.valueOf

[David Holmes]

On 17/04/2021 4:54 am, Raffaello Giulietti wrote:
I guess the reporter meant to limit the cache range similarly to the one 
used for valueOf().


I have no clue about the benefit/cost ratio for the proposed String 
cache. It really depends on usage, workload, etc. One can easily imagine 
both extreme scenarios but it's hard to tell how the average one would 
look.


My post is only about either solving the issue by implementing the 
cache, which I can contribute to; or closing it because of lack of 
real-world need or interest.


Caching for the sake of caching is not an objective in itself. Unless 
the caching can be shown to solve a real problem, and the strategy for 
managing the cache is well-defined, then I would just close the 
enhancement request. (Historically whether an issue we don't have any 
firm plans to address is just left open "forever" or closed, depends 
very much on who does the bug triaging in that area. :) )


Cheers,
David



Greetings
Raffaello


On 2021-04-16 20:36, Roger Riggs wrote:

Hi,

Is there any way to quantify the savings?
And what technique can be applied to limit the size of the cache.
The size of the small integer cache is somewhat arbitrary.

Regards, Roger

On 4/16/21 12:48 PM, Raffaello Giulietti wrote:

Hello,

does the enhancement proposed in [1] make sense, both today and when 
wrappers will be migrated to primitive classes?

If so, it should be rather simple to add it and I could prepare a PR.
If not, the issue might perhaps be closed.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8252827

Re: 8252827: Caching Integer.toString just like Integer.valueOf

[Raffaello Giulietti]

I guess the reporter meant to limit the cache range similarly to the one 
used for valueOf().


I have no clue about the benefit/cost ratio for the proposed String 
cache. It really depends on usage, workload, etc. One can easily imagine 
both extreme scenarios but it's hard to tell how the average one would look.


My post is only about either solving the issue by implementing the 
cache, which I can contribute to; or closing it because of lack of 
real-world need or interest.



Greetings
Raffaello


On 2021-04-16 20:36, Roger Riggs wrote:

Hi,

Is there any way to quantify the savings?
And what technique can be applied to limit the size of the cache.
The size of the small integer cache is somewhat arbitrary.

Regards, Roger

On 4/16/21 12:48 PM, Raffaello Giulietti wrote:

Hello,

does the enhancement proposed in [1] make sense, both today and when 
wrappers will be migrated to primitive classes?

If so, it should be rather simple to add it and I could prepare a PR.
If not, the issue might perhaps be closed.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8252827

Re: 8252827: Caching Integer.toString just like Integer.valueOf

[Roger Riggs]

Hi,

Is there any way to quantify the savings?
And what technique can be applied to limit the size of the cache.
The size of the small integer cache is somewhat arbitrary.

Regards, Roger

On 4/16/21 12:48 PM, Raffaello Giulietti wrote:

Hello,

does the enhancement proposed in [1] make sense, both today and when 
wrappers will be migrated to primitive classes?

If so, it should be rather simple to add it and I could prepare a PR.
If not, the issue might perhaps be closed.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8252827