<div dir="ltr"><div>Hi Staffan,</div><div><br></div>Grant and the guys will correct me if I'm wrong, but pull requests are definitely preferred. I think submitting/creating/fiding an issue on the github repo and then linking the pull request is the correct approach.<div>I am interested in the improvements you have mentioned, as I know I might be doing some performance requirements testing soon using quickfix/n for an up and coming project. I know that the quickfix/n project has not specifically focused much on performance, more on feature completeness, so I'm sure your suggestions will be welcomed.</div><div><br></div><div>Ruaan</div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature"><font face="arial, helvetica, sans-serif">Ruaan Viljoen</font><div><br></div></div></div>
<br><div class="gmail_quote">On 5 November 2014 15:09, Staffan Ulfberg <span dir="ltr"><<a href="mailto:staffan@ulfberg.se" target="_blank">staffan@ulfberg.se</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div>Hi,<br><br></div>When passing data from SocketReader (and also from SocketInitiatorThread -- I haven't studied the code enough to really understand why there are two copies of the code that performs the approximate same thing).<br><br></div><div>I would propose to change the code that calls parser_.AddToStream() to use the other overload of that method that takes a byte[] and a count instead of a string as arguments. (And also to remove the ref keyword on the byte[] argument which is not needed and therefore confusing.)<br><br></div><div>The main reason, and the cause for me starting to read the code in the first place, is that currently characters that use several bytes in UTF-8 are handled incorrectly if a socket read boundary gets between the bytes forming a character. If we really want to use the Parser.AddToSteam(string) method, it is still possible by using a System.Text.Decoder instance that keeps state between successive calls to remember that part of a character could not be decoded.<br><br>I made that change but then saw that the first thing that happens in Parser is that the string is converted back to bytes anyway. So, a much simpler approach seems to be to let the SocketReader just care about bytes, and to let the Parser take care of any encoding issues. I did that (I just scrapped the Parser.AddToStream() method and fixed the compilation errors) and now everything works much better. (We had random problems when having strings containing non-ascii characters like ö and ß.)<br></div><div><br></div><div>As a side note, looking at the Parser code there is a lot that could be done to increase efficiency. As it is now the buffered byte array is converted into a string more than once is various methods. I will probably fix that if we continue to use quickfixn. What is the proposed way to work with changing the code? Should I start a new branch on github and submit pull requests, or should I just post patches to this list, at least as long as there are few changes? Anyway, for the multi byte character boundary bug the diff is attached below. Please tell me if a pull request is preferred.<br></div><div><br></div>Staffan<br><br><br>diff --git a/QuickFIXn/Parser.cs b/QuickFIXn/Parser.cs<br>index 8bf706b..3f6dd27 100755<br>--- a/QuickFIXn/Parser.cs<br>+++ b/QuickFIXn/Parser.cs<br>@@ -6,7 +6,7 @@ namespace QuickFix<br> {<br> private byte[] buffer_ = new byte[512];<br> int usedBufferLength = 0;<br>- public void AddToStream(ref byte[] data, int bytesAdded)<br>+ public void AddToStream(byte[] data, int bytesAdded)<br> {<br> if (buffer_.Length < usedBufferLength + bytesAdded)<br> System.Array.Resize<byte>(ref buffer_, (usedBufferLength + bytesAdded));<br>@@ -17,7 +17,7 @@ namespace QuickFix<br> public void AddToStream(string data)<br> {<br> byte[] bytes = System.Text.Encoding.UTF8.GetBytes(data);<br>- AddToStream(ref bytes, bytes.Length);<br>+ AddToStream(bytes, bytes.Length);<br> <br> }<br> <br>diff --git a/QuickFIXn/SocketInitiatorThread.cs b/QuickFIXn/SocketInitiatorThread.cs<br>index 7a7e0a0..c196c54 100755<br>--- a/QuickFIXn/SocketInitiatorThread.cs<br>+++ b/QuickFIXn/SocketInitiatorThread.cs<br>@@ -79,7 +79,7 @@ namespace QuickFix<br> {<br> int bytesRead = ReadSome(readBuffer_, 1000);<br> if (bytesRead > 0)<br>- parser_.AddToStream(System.Text.Encoding.UTF8.GetString(readBuffer_, 0, bytesRead));<br>+ parser_.AddToStream(readBuffer_, bytesRead); <br> else if (null != session_)<br> {<br> session_.Next();<br>diff --git a/QuickFIXn/SocketReader.cs b/QuickFIXn/SocketReader.cs<br>index 9e2f475..f977ed8 100755<br>--- a/QuickFIXn/SocketReader.cs<br>+++ b/QuickFIXn/SocketReader.cs<br>@@ -43,7 +43,7 @@ namespace QuickFix<br> {<br> int bytesRead = ReadSome(readBuffer_, timeoutMilliseconds: 1000);<br> if (bytesRead > 0)<br>- parser_.AddToStream(System.Text.Encoding.UTF8.GetString(readBuffer_, 0, bytesRead));<br>+ parser_.AddToStream(readBuffer_, bytesRead);<br> else if (null != qfSession_)<br> {<br> qfSession_.Next();<br><br></div>
<br>_______________________________________________<br>
Quickfixn mailing list<br>
<a href="mailto:Quickfixn@lists.quickfixn.com">Quickfixn@lists.quickfixn.com</a><br>
<a href="http://lists.quickfixn.com/listinfo.cgi/quickfixn-quickfixn.com" target="_blank">http://lists.quickfixn.com/listinfo.cgi/quickfixn-quickfixn.com</a><br>
<br></blockquote></div><br></div>