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

StringBuilderAsBuffer #2

Open
ari opened this issue Mar 18, 2013 · 2 comments
Open

StringBuilderAsBuffer #2

ari opened this issue Mar 18, 2013 · 2 comments

Comments

@ari
Copy link
Collaborator

ari commented Mar 18, 2013

This is really bad. StringBuilder must not be used by multiple threads:

http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/StringBuilder.html

Java already has classes to do what you have reinvented from scratch such as... http://docs.oracle.com/javase/6/docs/api/java/io/FileWriter.html

If you used this, you would have no need to create multiple threads for reading and writing since Java will internally deal with the buffering for you.

@3awsomies
Copy link
Contributor

So, where are we with this? Do you want this done using existing classes? Some points to consider:

  1. Because we are using StringBuilder, that's one of the reasons why we synchronize the reads/writes externally.
  2. "Reinventing the code" - yes but worth given the performance gain and customization?

And also passing around the big buffer among 3 classes is intentional as it saves the object being copied to multiple memory areas and using that much extra space, especially when memory allocation can really make or break this application.

@ari
Copy link
Collaborator Author

ari commented Mar 19, 2013

OK, I see your synchronise around the StringBuilder. But I still don't understand the complexity of your code. Did you originally start with a FileWriter from Java and found the performance lacking? My understanding of FileWriter is that it is exactly what you wrote: a Java thread which handles writing, buffer locking between threads and has a variable size buffer.

If your code works, I'll merge it as is. But later on I'll probably rewrite it to use JDK classes and Apache libraries which have had thousands of hours of optimisation and testing effort. Unless you tell me that you already tried java.io.FileWriter and there were problems with it.

daczo added a commit that referenced this issue Apr 15, 2013
merging changes from ari's repo.
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

No branches or pull requests

2 participants