{{Quickfixn}} Passing data from SocketReader to Parser

[Ruaan Viljoen]

Hi Staffan,

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.
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.

Ruaan

Ruaan Viljoen


On 5 November 2014 15:09, Staffan Ulfberg <staffan at ulfberg.se> wrote:

> Hi,
>
> 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).
>
> 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.)
>
> 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.
>
> 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 ß.)
>
> 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.
>
> Staffan
>
>
> diff --git a/QuickFIXn/Parser.cs b/QuickFIXn/Parser.cs
> index 8bf706b..3f6dd27 100755
> --- a/QuickFIXn/Parser.cs
> +++ b/QuickFIXn/Parser.cs
> @@ -6,7 +6,7 @@ namespace QuickFix
>      {
>          private byte[] buffer_ = new byte[512];
>          int usedBufferLength = 0;
> -        public void AddToStream(ref byte[] data, int bytesAdded)
> +        public void AddToStream(byte[] data, int bytesAdded)
>          {
>              if (buffer_.Length < usedBufferLength + bytesAdded)
>                  System.Array.Resize<byte>(ref buffer_, (usedBufferLength
> + bytesAdded));
> @@ -17,7 +17,7 @@ namespace QuickFix
>          public void AddToStream(string data)
>          {
>              byte[] bytes = System.Text.Encoding.UTF8.GetBytes(data);
> -            AddToStream(ref bytes, bytes.Length);
> +            AddToStream(bytes, bytes.Length);
>
>          }
>
> diff --git a/QuickFIXn/SocketInitiatorThread.cs
> b/QuickFIXn/SocketInitiatorThread.cs
> index 7a7e0a0..c196c54 100755
> --- a/QuickFIXn/SocketInitiatorThread.cs
> +++ b/QuickFIXn/SocketInitiatorThread.cs
> @@ -79,7 +79,7 @@ namespace QuickFix
>              {
>                  int bytesRead = ReadSome(readBuffer_, 1000);
>                  if (bytesRead > 0)
> -
> parser_.AddToStream(System.Text.Encoding.UTF8.GetString(readBuffer_, 0,
> bytesRead));
> +                    parser_.AddToStream(readBuffer_,
> bytesRead);
>                  else if (null != session_)
>                  {
>                      session_.Next();
> diff --git a/QuickFIXn/SocketReader.cs b/QuickFIXn/SocketReader.cs
> index 9e2f475..f977ed8 100755
> --- a/QuickFIXn/SocketReader.cs
> +++ b/QuickFIXn/SocketReader.cs
> @@ -43,7 +43,7 @@ namespace QuickFix
>              {
>                  int bytesRead = ReadSome(readBuffer_,
> timeoutMilliseconds: 1000);
>                  if (bytesRead > 0)
> -
> parser_.AddToStream(System.Text.Encoding.UTF8.GetString(readBuffer_, 0,
> bytesRead));
> +                    parser_.AddToStream(readBuffer_, bytesRead);
>                  else if (null != qfSession_)
>                  {
>                      qfSession_.Next();
>
>
> _______________________________________________
> Quickfixn mailing list
> Quickfixn at lists.quickfixn.com
> http://lists.quickfixn.com/listinfo.cgi/quickfixn-quickfixn.com
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.quickfixn.com/pipermail/quickfixn-quickfixn.com/attachments/20141105/c445abb9/attachment-0001.htm>