Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 566685

Summary: More efficient write technique in SafeChunkyOutputStream.
Product: [Eclipse Project] Platform Reporter: Jeremy Whiting <jwhiting>
Component: ResourcesAssignee: Jeremy Whiting <jwhiting>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: akurtako, karsten.thoms, Lars.Vogel, loskutov
Version: 4.17   
Target Milestone: 4.18 M1   
Hardware: PC   
OS: Linux   
See Also: https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/168842
https://bugs.eclipse.org/bugs/show_bug.cgi?id=566983
https://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=46ccfcd6080964a9923a854609a750325f2342ce
Whiteboard:
Bug Depends on:    
Bug Blocks: 566485    
Attachments:
Description Flags
Primitive summary of stack frame durations. none

Description Jeremy Whiting CLA 2020-09-04 13:46:36 EDT
Using a tool called Byteman (byteman.jboss.org) the processing of stream write method calls indicates a source of inefficient processing.

 *** Slow write calls ***java.io.FilterOutputStream.write(FilterOutputStream.java:88)
org.eclipse.core.internal.localstore.SafeChunkyOutputStream.write(SafeChunkyOutputStream.java:95)
java.io.FilterOutputStream.write(FilterOutputStream.java:137)
java.io.DataOutputStream.write(DataOutputStream.java:107)
java.io.DataOutputStream.writeUTF(DataOutputStream.java:401)
java.io.DataOutputStream.writeUTF(DataOutputStream.java:323)
org.eclipse.core.internal.resources.MarkerWriter.snap(MarkerWriter.java:158)
org.eclipse.core.internal.resources.MarkerManager.snap(MarkerManager.java:581)
org.eclipse.core.internal.resources.SaveManager.lambda$3(SaveManager.java:1795)
org.eclipse.core.internal.watson.ElementTreeIterator.doIteration(ElementTreeIterator.java:85)

 What I propose is to enhance the technique of writing to the stream. To replace the individual write(int) calls with the bulk write(byte[], int, int) call.
 Doing that involves overriding the implementation of j.i.FilterOutputStream.write(byte[], int, int) with a call to write to encapsulated out field.

 I am expecting this change to improve efficiency but not change functionality. These small efficiency gains help improve the responsiveness of the Eclipse platform.

 Is there any interest in this proposal ?

 I've created a commit and pushed that into the Gerrit code review system. With this Change-Id: Ida150a45ecdc0917acb007cf1da49340fe144c13.

Regards,
Jeremy
Comment 1 Andrey Loskutov CLA 2020-09-04 13:56:24 EDT
Thanks, have you verified the change with some kind of performance testing? Ideally either as unit test or simple standalone example using this API.
Comment 2 Jeremy Whiting CLA 2020-09-07 05:12:59 EDT
 I'll get some JMH results and provide them here.
Comment 3 Jeremy Whiting CLA 2020-09-08 08:55:18 EDT
Created attachment 284081 [details]
Primitive summary of stack frame durations.
Comment 4 Jeremy Whiting CLA 2020-09-08 08:55:45 EDT
 A benchmark was created to compare the difference. These are the headline results. An attachment shows in more detail the time spent in the call stack.

Benchmark                     Mode  Cnt   Score   Error   Units
MyBenchmark.original         thrpt    5  18.339 ± 0.359  ops/ms
MyBenchmark.original:·stack  thrpt          NaN             ---
MyBenchmark.proposed         thrpt    5  26.803 ± 0.193  ops/ms
MyBenchmark.proposed:·stack  thrpt          NaN             ---

 The call stack of the original shows the write method taking 30.9%.
 The benchmark can be downloaded here [1].

[1] https://gitlab.com/whitingjr/jdt-resources-jmh-java
Comment 5 Karsten Thoms CLA 2020-09-09 19:46:08 EDT
Looks promising!
Comment 6 Lars Vogel CLA 2020-09-10 02:11:44 EDT
Thanks Jeremy for looking into this. Should a similar change be done for SafeFileOutputStream? And on the input side, what about SafeChunkyInputStream and SafeFileInputStream?
Comment 7 Lars Vogel CLA 2020-09-15 08:19:59 EDT
Thanks Jeremy.
Comment 9 Jeremy Whiting CLA 2020-09-21 08:30:31 EDT
 The three classes you suggested are good candidates.
 Shall I create a separate ticket ?
Comment 10 Lars Vogel CLA 2020-09-21 08:45:05 EDT
(In reply to Jeremy Whiting from comment #9)
>  The three classes you suggested are good candidates.
>  Shall I create a separate ticket ?

I created already tickets and implemented some prototypes, would be great if you can take them over. Is it OK to assign the bugs to you?