search

MetacoSA / NBitcoin

Comprehensive Bitcoin library for the .NET framework.
MIT License
1.86k stars 844 forks source link

`ReadEx` in .NET 6 #1088

Closed kiminuo closed 2 years ago

kiminuo commented 2 years ago

The idea of this PR is again to rely on .NET 6 implementation of ReadAsync instead of relying on BeginRead and EndRead.

kiminuo commented 2 years ago

@NicolasDorier Would this make sense to you?

kiminuo commented 2 years ago

@NicolasDorier Friendly ping

kiminuo commented 2 years ago

@NicolasDorier Friendly ping

NicolasDorier commented 2 years ago

@kiminuo sorry checking now

NicolasDorier commented 2 years ago

@kiminuo I don't like this, reason is that while before I was expecting the BitcoinStream to be read over a network stream, I backed away from this, and now advise to use it solely to deserialize from memory. This is actually how NBitcoin's Node class itself do it: First taking the whole message in a buffer, then parse the buffer.

  1. Using ReadAsync over a Read is an anti pattern with non obvious consequences which can cause thread starvation in some situation.
  2. ReadAsync degrades performance hugely in the main usecase I want BitcoinStream to support: deserializing from in memory buffer.