This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 199001 - Do not unnecessarily copy characters.
Summary: Do not unnecessarily copy characters.
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Execution (show other bugs)
Version: 7.0
Hardware: PC Linux
: P2 normal (vote)
Assignee: Martin Entlicher
URL:
Keywords: PERFORMANCE
Depends on:
Blocks:
 
Reported: 2011-05-30 13:35 UTC by Martin Entlicher
Modified: 2011-06-02 21:07 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Entlicher 2011-05-30 13:35:06 UTC
org.netbeans.core.execution.OutputStreamWriter.write(OutputStreamWriter.java:117) copies the String into an array of characters and sends it to the print stream.
But in the majority of cases off == 0 and len == str.length(). Then the copy just hurts performance and memory and the String can be printed directly.

Discovered during the evaluation of issue #197600.
Comment 1 Martin Entlicher 2011-05-30 14:11:34 UTC
Fixed by changeset:   194841:d20ba1cae859
and merged into release701 branch as changeset:   199558:ec996ddc406
Comment 2 err 2011-05-31 14:39:18 UTC
(In reply to comment #1)
> Fixed by changeset:   194841:d20ba1cae859
> and merged into release701 branch as changeset:   199558:ec996ddc406

I think this change introduces a bug.

I guess the idea with the copy is to funnel all the output through one queue method; With this change, if you execute:
    System.err.println("SYSTEM ERR OUTPUT");
    System.err.print("Char array to stderr.\n".toCharArray());
the output is not necessarily in the correct order, since the two statements do not go through the same queueing mechanism.

Of course the queue/flush problem in bug #197600 needs to get fixed.
Comment 3 Martin Entlicher 2011-05-31 15:29:26 UTC
I'm sorry, I did not get your point. What "queueing mechanism" are you talking about?
Both methods call write() on the underlying writer and under a synchronization on a lock.

But looking into the code is always beneficent, I've found out that there's a missing synchronization in OutputStreamWriter.write(String str, int off, int len) method.
Jesse, as the original author, please correct me if I'm wrong.
Comment 4 err 2011-05-31 16:06:20 UTC
> I did not get your point. What "queueing mechanism"

Since I didn't look at any code, I could be totally wrong.

According to Yarda, if things are working correctly there could be up to a 5 second delay. Before the change, all the output was stalled. Now you say some data gets output and some doesn't. That makes me think that after the other bug is fixed, some output would happen right away and some would be delayed by up to 5 seconds.

My point is that all the data should be treated the same.
Comment 5 Martin Entlicher 2011-05-31 16:20:50 UTC
The code seems to be really not finished, I'll add the synchronization and a check for closed stream. ensureOpen() method is not called anywhere...

I have no idea where the 5 second delay is implemented. I do not see any more issues in the OutputStreamWriter class.
Comment 6 Quality Engineering 2011-05-31 19:25:12 UTC
Integrated into 'main-golden', will be available in build *201105310954* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/d20ba1cae859
User: mentlicher@netbeans.org
Log: #199001 Performance improved by not copying the output characters when not necessary.
Comment 7 Jesse Glick 2011-05-31 22:00:50 UTC
(In reply to comment #3)
> I've found out that there's a
> missing synchronization in OutputStreamWriter.write(String str, int off, int
> len) method.

It rather seems to me that write(char[],int,int) does _not_ need synchronization - PrintStream.print does its own synch, and 'synchronized (lock) {...}' in OSW seems to be used just to guard whether 'out' is null ~ closed or not.

> Jesse, as the original author, please correct me if I'm wrong.

I do not think I am the original author, though I do not know who is.
Comment 8 Quality Engineering 2011-06-01 13:30:18 UTC
Integrated into 'main-golden', will be available in build *201106010401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/d20ba1cae859
User: mentlicher@netbeans.org
Log: #199001 Performance improved by not copying the output characters when not necessary.
Comment 9 Martin Entlicher 2011-06-01 14:10:58 UTC
Hmm, I'd rather leave the synchronization there, changes like this go actually beyond what I wanted to do.
If I remove the synchronization, then out.print() can be called on a closed print stream, which will result in a silent failure. We can check for the failure by out.checkError(), but this method flushes the output. I prefer not to do changes like this.

http://hg.netbeans.org/main/rev/7bf3ac9904d3
http://hg.netbeans.org/releases/rev/3431fda7e99c
Comment 10 err 2011-06-01 14:31:43 UTC
Assuming bug #197600 gets fixed, I hope there's no funny interactions.
Comment 11 Martin Entlicher 2011-06-01 14:53:01 UTC
There should not be. This class just delegates to a PrintStream, nothing more.
As a side-effect, it will fix your issue (#197600) with writing Strings to messages.log file. Printing arrays of characters will not call flush, therefore it could not be seen until a flush is called (e.g. by printing a String). :-)
Comment 12 err 2011-06-01 15:37:23 UTC
> it will fix your issue (#197600)

I know (and I thank you for that). I must admit I'm quite curious as to what's going on and why this change partially fixes the other problem. I guess I'm not curious enough to dig through code...
Comment 13 Quality Engineering 2011-06-02 21:07:21 UTC
Integrated into 'main-golden', will be available in build *201106021001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/d20ba1cae859
User: mentlicher@netbeans.org
Log: #199001 Performance improved by not copying the output characters when not necessary.