> > On Tue, Jan 16, 2018 at 02:18:15PM +0000, Frediano Ziglio wrote: > > In order to use new TCP_CORK feature disable auto flush. > > 'the new TCP_CORK feature, disable auto flush' > Might be worth explaining in the commit log why you disable auto_flush > only for RedCommonGraphicsChannel. > Agreed. Mainly it could be that the channel push an item at a time in the network having the opposite result. Quite unlikely to be honest but tested that these channels works as expected. > > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > > --- > > server/common-graphics-channel.c | 17 +++++++++-------- > > server/red-channel-client.c | 1 + > > 2 files changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/server/common-graphics-channel.c > > b/server/common-graphics-channel.c > > index ce6b5e57..083ab3eb 100644 > > --- a/server/common-graphics-channel.c > > +++ b/server/common-graphics-channel.c > > @@ -83,14 +83,15 @@ bool > > common_channel_client_config_socket(RedChannelClient *rcc) > > > > // TODO - this should be dynamic, not one time at channel creation > > is_low_bandwidth = main_channel_client_is_low_bandwidth(mcc); > > - /* FIXME: Using Nagle's Algorithm can lead to apparent delays, > > depending > > - * on the delayed ack timeout on the other side. > > - * Instead of using Nagle's, we need to implement message buffering on > > - * the application level. > > - * see: http://www.stuartcheshire.org/papers/NagleDelayedAck/ > > - */ > > - red_stream_set_no_delay(stream, !is_low_bandwidth); > > - > > + if (!red_stream_set_auto_flush(stream, false)) { > > + /* FIXME: Using Nagle's Algorithm can lead to apparent delays, > > depending > > + * on the delayed ack timeout on the other side. > > + * Instead of using Nagle's, we need to implement message > > buffering on > > + * the application level. > > + * see: http://www.stuartcheshire.org/papers/NagleDelayedAck/ > > + */ > > + red_stream_set_no_delay(stream, !is_low_bandwidth); > > + } > > // TODO: move wide/narrow ack setting to red_channel. > > red_channel_client_ack_set_client_window(rcc, > > is_low_bandwidth ? > > diff --git a/server/red-channel-client.c b/server/red-channel-client.c > > index f154c5c6..32ac30d1 100644 > > --- a/server/red-channel-client.c > > +++ b/server/red-channel-client.c > > @@ -1328,6 +1328,7 @@ void red_channel_client_push(RedChannelClient *rcc) > > if ((red_channel_client_no_item_being_sent(rcc) && > > g_queue_is_empty(&rcc->priv->pipe)) || > > red_channel_client_waiting_for_ack(rcc)) { > > red_channel_client_watch_update_mask(rcc, SPICE_WATCH_EVENT_READ); > > + red_stream_flush(rcc->priv->stream); > > I would add a rationale in the commit log regarding why the > red_stream_flush() is in this specific place. > Maybe better to add a comment to code. Maybe this hunk should be moved to 1/3 ? > Christophe > Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel