Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework append for better memory efficiency #1156

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MiguelAMarcelino
Copy link

Trying to optimize the append method to improve memory efficiency and reduce object allocations. Previously, the code relied on the textWithoutEscapeChar method, which involved converting the internal StringBuilder to a String and then to a char[]. My goal here is to reduce unnecessary intermediate allocations and processing overhead.

Changes Introduced

  • Streamlined operations to minimize the number of object switches and allocations during the append process.
  • Removed textWithoutEscapeChar to avoid the conversion chain (StringBuilderStringchar[]).

Alternative Consideration:

An alternative approach considered was to rewrite the textWithoutEscapeChar method for better performance. The revised method would look like this:

public char[] textWithoutEscapeChar() {
  char[] result = new char[text.length()];
  text.getChars(0, text.length(), result, 0);
  return result;
}

This version avoids converting the StringBuilder to a String and directly copies the characters into a char[], reducing unnecessary overhead.

Future Enhancements:

There is a potential case to investigate the frequent resizing of the StringBuilder used in Text. Pre-sizing the StringBuilder to an appropriate capacity may further improve performance and memory usage.

@MiguelAMarcelino
Copy link
Author

MiguelAMarcelino commented Jan 29, 2025

@jknack hi 👋.
Just wanted to be sure I am following the guidelines for opening a PR on your repo.
Please check whenever you have time. I am also open to discuss these changes.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant