replaced stream with memoryStream for reading the message.

[akshaybheda]

No description provided.

[akshaybheda]

#1026

[akshaybheda]

@andersjonsson this is the only way i could find to dispose the memory stream

[andersjonsson]

Yeah that's not too pretty.

I just realized that there might be a simpler way to fix this.
If I'm not mistaken message.CreateBufferedCopy(int.MaxValue); creates an in-memory-copy of the message.
That should reduce the blocking by a lot. It's not async but it's more straight forward code.

[akshaybheda]

aren't we creating the message after we create the reader, in the readMessageAsync, where do you propose the message.CreateBufferedCopy(int.MaxValue);

[akshaybheda]

do you mean in ProcessMessage, we would add

				MessageBuffer mb = requestMessage.CreateBufferedCopy(int.MaxValue);
				Message responseMsg = mb.CreateMessage();
				requestMessage = mb.CreateMessage();

above
reader = requestMessage.GetReaderAtBodyContents();

[andersjonsson]

do you mean in ProcessMessage

My, untested, idea was to do it in ReadMessageAsync

Message message = Message.CreateMessage(reader, maxSizeOfHeaders, MessageVersion).CreateBufferedCopy(int.MaxValue).CreateMessage();

return Task.FromResult(message);

It's still blocking but it will read the message immediately, thus the blocking should be really short.

That approach could be paired with a memorystream for a simpler async version (since the memorystream can be disposed right after the creation of the buffered copy), but that will copy the request data one more time, resulting in higher memory usage.

[akshaybheda]

I guess this could work, only thing i am worried about is even its blocking for short period, it can still cause threadpool queue for wait, but i can test this in prod and let you know if this change made any better

[akshaybheda]

also, once you merge this can you create a new version?

[andersjonsson]

Is it possible for you to use a locally linked .dll in prod? I'd rather not deploy this to nuget before we know that it's an improvement.

[akshaybheda]

It would be tricky for me to do that considering all the deployment policies, any other way you could release a nuget with a named version or something?

[akshaybheda]

I cannot checkin the whole soapcore and get to the release stage, it could violate some policies here, only way is using nuget

[andersjonsson]

I cannot checkin the whole soapcore and get to the release stage, it could violate some policies here, only way is using nuget

Ok, I thought that maybe you could compile your own version of soapcore and include the dll in your builds.
I'm not sure what to do actually.

Ran some benchmarks for three implementations. The original one, the one you have, and one that reads the request to a memorystream asynchronously and then creates a buffered copy.

| Method                               | Mean     | Error    | StdDev   | Median   | Gen0    | Gen1   | Allocated |
|------------------------------------- |---------:|---------:|---------:|---------:|--------:|-------:|----------:|
| ReadMessageMemoryStreamBufferedAsync | 27.40 μs | 0.511 μs | 0.478 μs | 27.25 μs | 13.3362 |      - |  54.56 KB |
| ReadMessageBufferedAsync             | 29.16 μs | 0.671 μs | 1.968 μs | 28.35 μs | 13.1226 | 0.0610 |   53.7 KB |
| ReadMessageOriginalAsync             | 10.35 μs | 0.202 μs | 0.248 μs | 10.29 μs |  5.4169 |      - |  22.15 KB |

The buffered ones are a lot slower and uses a lot more memory. I think that there could be benefits to the throughput but it's hard to be sure.

I think, based on this test, that I'd be ok with giving the version with memorystream a shot. That one avoids blocking and doesn't seem to perform much worse than the blocking one.
I'm thinking something like this:

public async Task<Message> ReadMessageAsync(Stream stream, int maxSizeOfHeaders, string contentType)
{
	if (stream == null)
	{
		throw new ArgumentNullException(nameof(stream));
	}

	Message message;

	using (var ms = new MemoryStream())
	{
		await stream.CopyToAsync(ms);
		ms.Seek(0, SeekOrigin.Begin);
		XmlReader reader;

		var readEncoding = SoapMessageEncoderDefaults.ContentTypeToEncoding(contentType);

		if (readEncoding == null)
		{
			// Fallback to default or writeEncoding
			readEncoding = _writeEncoding;
		}

		var supportXmlDictionaryReader = SoapMessageEncoderDefaults.TryValidateEncoding(readEncoding, out _);

		if (supportXmlDictionaryReader)
		{
			reader = XmlDictionaryReader.CreateTextReader(ms, readEncoding, ReaderQuotas, dictionaryReader => { });
		}
		else
		{
			var streamReaderWithEncoding = new StreamReader(ms, readEncoding);
			var xmlReaderSettings = new XmlReaderSettings() { IgnoreWhitespace = true, DtdProcessing = DtdProcessing.Prohibit, CloseInput = true };
			reader = XmlReader.Create(streamReaderWithEncoding, xmlReaderSettings);
		}

		message = Message.CreateMessage(reader, maxSizeOfHeaders, MessageVersion).CreateBufferedCopy(int.MaxValue).CreateMessage();
	}

	return message;
}

What do you think? If you agree, please change your PR and I'll merge it

[akshaybheda]

this looks good, will modify

[andersjonsson]

Thanks! We'll see how this goes

I'll publish new release to nuget

[akshaybheda]

Thanks for all the efforts!! appreciate it. Will dig deeper for content length thing. and will also let you know about the performance