You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gary Gregory <ga...@gmail.com> on 2024/10/28 22:02:00 UTC
Re: (commons-text) 01/04: Removed the final modifier from the static methods
Hello,
Would you please explain why you need this change?
TY,
Gary
On Sat, Oct 26, 2024 at 5:12 AM <eb...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> ebourg pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/commons-text.git
>
> commit 31de78423c7f38e7fbdf4f0dd1c618e54a150df2
> Author: Emmanuel Bourg <eb...@apache.org>
> AuthorDate: Sat Oct 26 10:35:13 2024 +0200
>
> Removed the final modifier from the static methods
> ---
> .../org/apache/commons/text/StringEscapeUtils.java | 30
> +++++++++++-----------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/src/main/java/org/apache/commons/text/StringEscapeUtils.java
> b/src/main/java/org/apache/commons/text/StringEscapeUtils.java
> index 1a6bd9b7..5a531bd8 100644
> --- a/src/main/java/org/apache/commons/text/StringEscapeUtils.java
> +++ b/src/main/java/org/apache/commons/text/StringEscapeUtils.java
> @@ -506,7 +506,7 @@ public class StringEscapeUtils {
> * @return The input String, enclosed in double quotes if the value
> contains a comma,
> * newline or double quote, {@code null} if null string input
> */
> - public static final String escapeCsv(final String input) {
> + public static String escapeCsv(final String input) {
> return ESCAPE_CSV.translate(input);
> }
>
> @@ -540,7 +540,7 @@ public class StringEscapeUtils {
> * @param input String to escape values in, may be null
> * @return String with escaped values, {@code null} if null string
> input
> */
> - public static final String escapeEcmaScript(final String input) {
> + public static String escapeEcmaScript(final String input) {
> return ESCAPE_ECMASCRIPT.translate(input);
> }
>
> @@ -552,7 +552,7 @@ public class StringEscapeUtils {
> * @param input the {@code String} to escape, may be null
> * @return a new escaped {@code String}, {@code null} if null string
> input
> */
> - public static final String escapeHtml3(final String input) {
> + public static String escapeHtml3(final String input) {
> return ESCAPE_HTML3.translate(input);
> }
>
> @@ -582,7 +582,7 @@ public class StringEscapeUtils {
> * @see <a href="http://www.w3.org/TR/html401/charset.html#h-5.3">HTML
> 4.01 Character References</a>
> * @see <a href="
> http://www.w3.org/TR/html401/charset.html#code-position">HTML 4.01 Code
> positions</a>
> */
> - public static final String escapeHtml4(final String input) {
> + public static String escapeHtml4(final String input) {
> return ESCAPE_HTML4.translate(input);
> }
>
> @@ -607,7 +607,7 @@ public class StringEscapeUtils {
> * @param input String to escape values in, may be null
> * @return String with escaped values, {@code null} if null string
> input
> */
> - public static final String escapeJava(final String input) {
> + public static String escapeJava(final String input) {
> return ESCAPE_JAVA.translate(input);
> }
>
> @@ -634,7 +634,7 @@ public class StringEscapeUtils {
> * @param input String to escape values in, may be null
> * @return String with escaped values, {@code null} if null string
> input
> */
> - public static final String escapeJson(final String input) {
> + public static String escapeJson(final String input) {
> return ESCAPE_JSON.translate(input);
> }
>
> @@ -715,7 +715,7 @@ public class StringEscapeUtils {
> * @param input String to escape values in, may be null
> * @return String with escaped values, {@code null} if null string
> input
> */
> - public static final String escapeXSI(final String input) {
> + public static String escapeXSI(final String input) {
> return ESCAPE_XSI.translate(input);
> }
>
> @@ -739,7 +739,7 @@ public class StringEscapeUtils {
> * @return The input String, with enclosing double quotes removed and
> embedded double
> * quotes unescaped, {@code null} if null string input
> */
> - public static final String unescapeCsv(final String input) {
> + public static String unescapeCsv(final String input) {
> return UNESCAPE_CSV.translate(input);
> }
>
> @@ -754,7 +754,7 @@ public class StringEscapeUtils {
> * @param input the {@code String} to unescape, may be null
> * @return A new unescaped {@code String}, {@code null} if null
> string input
> */
> - public static final String unescapeEcmaScript(final String input) {
> + public static String unescapeEcmaScript(final String input) {
> return UNESCAPE_ECMASCRIPT.translate(input);
> }
>
> @@ -766,7 +766,7 @@ public class StringEscapeUtils {
> * @param input the {@code String} to unescape, may be null
> * @return a new unescaped {@code String}, {@code null} if null
> string input
> */
> - public static final String unescapeHtml3(final String input) {
> + public static String unescapeHtml3(final String input) {
> return UNESCAPE_HTML3.translate(input);
> }
>
> @@ -785,7 +785,7 @@ public class StringEscapeUtils {
> * @param input the {@code String} to unescape, may be null
> * @return a new unescaped {@code String}, {@code null} if null
> string input
> */
> - public static final String unescapeHtml4(final String input) {
> + public static String unescapeHtml4(final String input) {
> return UNESCAPE_HTML4.translate(input);
> }
>
> @@ -798,7 +798,7 @@ public class StringEscapeUtils {
> * @param input the {@code String} to unescape, may be null
> * @return a new unescaped {@code String}, {@code null} if null
> string input
> */
> - public static final String unescapeJava(final String input) {
> + public static String unescapeJava(final String input) {
> return UNESCAPE_JAVA.translate(input);
> }
>
> @@ -813,7 +813,7 @@ public class StringEscapeUtils {
> * @param input the {@code String} to unescape, may be null
> * @return A new unescaped {@code String}, {@code null} if null
> string input
> */
> - public static final String unescapeJson(final String input) {
> + public static String unescapeJson(final String input) {
> return UNESCAPE_JSON.translate(input);
> }
>
> @@ -833,7 +833,7 @@ public class StringEscapeUtils {
> * @see #escapeXml10(String)
> * @see #escapeXml11(String)
> */
> - public static final String unescapeXml(final String input) {
> + public static String unescapeXml(final String input) {
> return UNESCAPE_XML.translate(input);
> }
>
> @@ -844,7 +844,7 @@ public class StringEscapeUtils {
> * @param input the {@code String} to unescape, may be null
> * @return a new unescaped {@code String}, {@code null} if null
> string input
> */
> - public static final String unescapeXSI(final String input) {
> + public static String unescapeXSI(final String input) {
> return UNESCAPE_XSI.translate(input);
> }
>
>
>
Re: (commons-text) 01/04: Removed the final modifier from the static methods
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 29/10/2024 à 11:04, Dávid Szigecsán a écrit :
> It is true, that static methods can't be overridden in a subclass, however
> they can be hidden. So final keyword prevents method hiding.
That's correct. However StringEscapeUtils is an utils class with only
static methods and is not meant to be subclassed (I wonder why it isn't
final btw), so the final modifier is useless.
> Finally, the final keyword can slightly improve performance because the JVM
> should not check if the method is hidden or overridden in a subclass.
There is no polymorphism for static methods, the compiler resolves the
static method call depending on the type of the instance, and the
bytecode generated has an invokestatic instruction pointing directly to
the right method, unlike invokevirtual instructions. There is no runtime
impact.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: (commons-text) 01/04: Removed the final modifier from the static methods
Posted by Dávid Szigecsán <si...@gmail.com>.
Hi,
It is true, that static methods can't be overridden in a subclass, however
they can be hidden. So final keyword prevents method hiding.
It also helps make the code explicit.
Finally, the final keyword can slightly improve performance because the JVM
should not check if the method is hidden or overridden in a subclass.
Just two cents.
Regards,
David
Emmanuel Bourg <eb...@apache.org> ezt írta (időpont: 2024. okt. 29., K,
8:30):
> Hi Gary,
>
> Le 28/10/2024 à 23:02, Gary Gregory a écrit :
>
> > Would you please explain why you need this change?
> >
> > TY,
> > Gary
>
> static methods are not inherited and can't be overridden in a subclass.
> The 'final' modifier on a static method has no effect and can be removed.
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: (commons-text) 01/04: Removed the final modifier from the static methods
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 29/10/2024 à 11:55, Gary Gregory a écrit :
> In addition, this change prevents the JVM from inlining
> the code in some cases, see [1].
Interesting point, thank you for mentioning it. According to [1] method
inlining applies to static, final or private methods. The article has an
example with a static, non-final method.
I tested the inlining with the following code to see if there is a
difference between static and static final methods:
public class InliningTest {
public static void main(String[] args) {
long sum = 0;
for (int i = 0; i < 1000000; i++) {
sum += foo(i);
sum += bar(i);
}
System.out.println(sum);
}
public static long foo(int n) {
return n + 1;
}
public static final long bar(int n) {
return n + 1;
}
}
This was executed with Java 8 on Windows 10:
java -XX:+PrintCompilation -XX:+UnlockDiagnosticVMOptions
-XX:+PrintInlining InliningTest
And here is the result:
187 32 % 4 InliningTest::main @ 4 (38 bytes)
@ 12 InliningTest::foo (5 bytes) inline (hot)
@ 19 InliningTest::bar (5 bytes) inline (hot)
Both methods were inlined at the same time. I got similar results with
Java 17 and 23. I tried with a private method and it was also inlined at
the same time.
So it looks like the final modifier doesn't affect the inlining of
static methods, what matters is if the method can be inlined (static,
final or private method with a size under the configured threshold) and
the number of calls to the method.
Emmanuel Bourg
[1] https://www.baeldung.com/jvm-method-inlining
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: (commons-text) 01/04: Removed the final modifier from the static methods
Posted by Gary Gregory <ga...@gmail.com>.
On Tue, Oct 29, 2024 at 3:30 AM Emmanuel Bourg <eb...@apache.org> wrote:
> Hi Gary,
>
> Le 28/10/2024 à 23:02, Gary Gregory a écrit :
>
> > Would you please explain why you need this change?
> >
> > TY,
> > Gary
>
> static methods are not inherited and can't be overridden in a subclass.
> The 'final' modifier on a static method has no effect and can be removed.
>
That's not how Java works [1].
A static final method cannot be overridden, you cannot redefine it in a
subclass as another static method with the same signature, the compiler
will not allow it. In addition, this change prevents the JVM from inlining
the code in some cases, see [1].
You should revert this change: Allowing subclasses to (unwisely) redefine
all these static methods is confusing and loses the performance
optimization the JVM can do.
The argument that the class itself could be made final would be for the
next major version.
Gary
[1]
https://docs.oracle.com/javase/specs/jls/se21/html/jls-8.html#jls-8.4.3.2
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: (commons-text) 01/04: Removed the final modifier from the static methods
Posted by Emmanuel Bourg <eb...@apache.org>.
Hi Gary,
Le 28/10/2024 à 23:02, Gary Gregory a écrit :
> Would you please explain why you need this change?
>
> TY,
> Gary
static methods are not inherited and can't be overridden in a subclass.
The 'final' modifier on a static method has no effect and can be removed.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org