[jira] [Comment Edited] (GROOVY-9637) Improve the performance of GString

["Paul King (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GROOVY-9637?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17156381#comment-17156381 ] 

Paul King edited comment on GROOVY-9637 at 7/12/20, 9:02 PM:
-------------------------------------------------------------

Since GStrings aren't themselves immutable, we probably have more work to do. E.g. while bad style and strongly discouraged, we don't disallow code like below:
{code}
def x = 42G
def y = "Answer is $x"
println y // => Answer is 42
y.strings[0] = '6 x 7 = '
println y // => 6 x 7 = 42
y.values[0] = 'the question'
println y // => 6 x 7 = the question
{code}
We could place in a change to make them immutable. That might have been a good candidate change for Groovy 3 but perhaps too late now unless discussed and agreed on mailing lists. Seems a better fit for Groovy 4?

Also, the PR doesn't taken into account if a metaclass is in play. Due to GROOVY-2599, this would take some work to be an issue (i.e. extremely rare).
E.g.:
{code}
def x = 42G
def y = "Answer is $x"
def z = "Answer is ${x.toString()}"
println y
println z
BigInteger.metaClass.toString {
    '' + delegate * 2
}
println 10G
println 10G.toString()
println y
println z
{code}
Due to GROOVY-2599, Groovy's InvokerHelper.format also ignores if a metaClass is in play for some hard-coded cases. But if we ever fixed GROOVY-2599, the behavior seen by the closure variant would also be seen by the non-closure variant. Which means the GString patch would turn that off. We could argue that isn't a big issue. Or turn it on only in CompileStatic code?

Perhaps worth discussing in mailing lists regardless. Others might have better suggestions. The idea is certainly nice.


was (Author: paulk):
Since GStrings aren't themselves immutable, we probably have more work to do. E.g.:
{code}
def x = 42G
def y = "Answer is $x"
println y
y.strings[0] = '6 x 7 = '
println y
y.values[0] = 'the question'
println y
{code}
We could place in a change to make them immutable. That might have been a good candidate change for Groovy 3 but perhaps too late now unless discussed and agreed on mailing lists. Seems a better fit for Groovy 4?

Also, the PR doesn't taken into account if a metaclass is in play. Due to GROOVY-2599, this would take some work to be an issue (i.e. extremely rare).
E.g.:
{code}
def x = 42G
def y = "Answer is $x"
def z = "Answer is ${x.toString()}"
println y
println z
BigInteger.metaClass.toString {
    '' + delegate * 2
}
println 10G
println 10G.toString()
println y
println z
{code}
Due to GROOVY-2599, Groovy's InvokerHelper.format also ignores if a metaClass is in play for some hard-coded cases. But if we ever fixed GROOVY-2599, the behavior seen by the closure variant would also be seen by the non-closure variant. Which means the GString patch would turn that off. We could argue that isn't a big issue. Or turn it on only in CompileStatic code?

Perhaps worth discussing in mailing lists regardless. Others might have better suggestions. The idea is certainly nice.

> Improve the performance of GString
> ----------------------------------
>
>                 Key: GROOVY-9637
>                 URL: https://issues.apache.org/jira/browse/GROOVY-9637
>             Project: Groovy
>          Issue Type: Improvement
>            Reporter: Daniel Sun
>            Priority: Major
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> {{GString}} will run {{toString()}} whenever its implementation needs its literal string, but unfortunately its literal string will be re-constructed for each time and the process costs quite a little of time.
> So I propose to check whether {{GString}} values are all of immutable type, e.g. primitive types and their boxed type, {{String}}, etc. If yes,  use the cached literal string, otherwise re-construct the literal string.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)