Enhance toString method in SockJsFrame

[polyglot-k]

Description:

• Removed usage of external libraries (StringUtils) and Math.min().
• Minimized intermediate String objects to reduce GC pressure.
• Used StringBuilder with pre-allocated capacity for better memory efficiency.
• Implemented manual handling of \n and \r replacements and truncated output after 80 characters.
• Ensured the method works efficiently for both short and long content strings.
• This change improves performance for high-frequency logging without affecting readability for standard logs.

Key Changes:

@Override
public String toString() {
  int maxLength = 80;
  int contentLength = this.content.length();
  int truncatedContentLength  = Math.min(contentLength, maxLength);
  
  int extra = (contentLength > maxLength ? truncatedSuffix.length() : 0);
  
  StringBuilder sb = new StringBuilder(truncatedContentLength  + extra);
  
  for (int i = 0; i < truncatedContentLength; i++) {
	  char c = this.content.charAt(i);
	  switch (c) {
		  case '\n' -> sb.append("\\n");
		  case '\r' -> sb.append("\\r");
		  default -> sb.append(c);
	  }
  }
  
  if (contentLength > maxLength) {
	  sb.append(truncatedSuffix);
  }
  
  return "SockJsFrame content='" + sb + "'";
}

Benefits:

• Reduced memory allocation and GC overhead.
• Avoids dependency on external libraries.
• Efficient handling for large strings.
• Preserves readability for logs.

[rstoyanchev]

Thanks for the pull request.

The title says "for better content representation", but it looks like the output doesn't change, or does it? In other words it's about performance and streamlined implementation?

[polyglot-k]

@rstoyanchev
Actually, the result is not changing. However, we changed it because using the "String" object directly is not good for GC.

[polyglot-k]

@rstoyanchev
I'm done reflecting the content

[bclozel]

I think the overall changes is making readability worse.
Instead of actively trying to guess the builder size in advance with multiple operations, I would favor using a default StringBuilder instance and use it all the way (instead of performing concatenations on the last line).

This would cause probably some more allocation, but in the end this code is not on a hot path so treating StringUtils as an "external library" and making maintenance worse is not worth it.

[bclozel]

Why not use the String Builder for this part as well? This PR is trying to optimize for memory allocation after all.

[polyglot-k]

Thank you for the review.

Regarding the other toString() methods, they also use StringBuilder, so I thought there’s no need for the unnecessary String objects to become GC targets in the JVM. Of course, I agree that the original code is more readable.

My perspective is that the readability of this part isn’t particularly high, and concatenating immutable Strings in this way is generally considered an anti-pattern, so I opted to use a mutable StringBuilder.

I will revise the return part as you suggested.

@bclozel

[rstoyanchev]

In the end I've gone for the simpler version of a fixed initialCapacity. The use of StringBuilder is good as it eliminates re-creating the String potentially multiple times. Given that SockJS frames typically have a \n that guarantees at least one replace and potentially more. The use of StringBuilder ensures the String will never be re-created more than once.

[polyglot-k]

@rstoyanchev thank u :)