Conerlius / protobuf-net

Automatically exported from code.google.com/p/protobuf-net
Other
0 stars 0 forks source link

Don't flush the underlying stream in ProtoWriter.Close #291

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
ProtoWriter.Close calls ProtoWriter.Flush which calls ((Stream)dest).Flush(). 
Flushing a stream is *never* necessary to make sure the data actually arrives. 
It is not required for correctness. 

Unfortunately, flushing a stream often has performance issues like forcing a 
disk-seek or forcing transmission of TCP packets. Flushing does not mean "make 
sure the data is not discarded on dispose". It means "do some physical action 
right now".

Is there a reason flushing is required here? If not, performance can be 
increased by removing it.

Original issue reported on code.google.com by abiturun...@gmail.com on 9 May 2012 at 1:59

GoogleCodeExporter commented 9 years ago
Addendum: This is not a theoretical issue. I just wrote a wrapper stream which 
ignores flush calls to work around this problem.

The Flush call was forcing FileStream to empty its internal write buffer 
leading to many calls to WriteFile with only 20 bytes or so. Performance killer.

Original comment by abiturun...@gmail.com on 9 May 2012 at 2:23

GoogleCodeExporter commented 9 years ago
Just want to let you know that this bug is still active in 2.0.0.622.

This bug is CRIPPLING performance when serializing many items to a FileStream. 
I'm surprised this bug was deemed "Medium" as it seems to be a MAJOR perf bug.

The screenshot shows performance when serializing many small items to an SSD 
(!) drive. We get 2% CPU usage and 529kb/sec disk speed. This is not right! One 
of the two resources should be maxed out (either CPU 12.5% or disk at 
200MB/sec).

There's nothing in the app throttling the process. Almost all time is spent in 
Flush.

Original comment by abiturun...@gmail.com on 30 Apr 2013 at 5:31

Attachments: