[Spice-devel] [spice-server v2 5/6] reds-stream: Cache TCP socket 'no-delay' value

[Uri Lublin]

On 03/10/2017 03:55 PM, Frediano Ziglio wrote:
>>
>> Rather than calling getsockopt when we need its value, we can cache it
>> as it can only be set through a call to reds_stream_set_no_delay()
>>
>> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
>
> I would avoid that
> - not in an hot path, it's just a system call;
> - with my patches this call is not necessary.
>
>> ---
>>  server/reds-stream.c | 16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/server/reds-stream.c b/server/reds-stream.c
>> index 1d815c1..66b1166 100644
>> --- a/server/reds-stream.c
>> +++ b/server/reds-stream.c
>> @@ -99,6 +99,7 @@ struct RedsStreamPrivate {
>>      ssize_t (*writev)(RedsStream *s, const struct iovec *iov, int iovcnt);
>>
>>      RedsState *reds;
>> +    bool no_delay;
>>  };
>>
>>  static ssize_t stream_write_cb(RedsStream *s, const void *buf, size_t size)
>> @@ -263,12 +264,17 @@ bool reds_stream_is_plain_unix(const RedsStream *s)
>>
>>  bool reds_stream_set_no_delay(RedsStream *stream, bool no_delay)
>>  {
>> -    return red_socket_set_no_delay(stream->socket, no_delay);
>> +    bool success;
>> +    success = red_socket_set_no_delay(stream->socket, no_delay);
>> +    if (success) {
>> +        stream->priv->no_delay = !!no_delay;
>> +    }
>> +    return success;
>>  }
>>
>>  int reds_stream_get_no_delay(RedsStream *stream)
>>  {
>> -    return red_socket_get_no_delay(stream->socket);
>> +    return stream->priv->no_delay;
>>  }
>>
>>  int reds_stream_send_msgfd(RedsStream *stream, int fd)
>> @@ -372,7 +378,13 @@ void reds_stream_push_channel_event(RedsStream *s, int
>> event)
>>
>>  static void reds_stream_set_socket(RedsStream *stream, int socket)
>>  {
>> +    int no_delay;
>>      stream->socket = socket;
>> +    no_delay = red_socket_get_no_delay(socket);
>> +    if (no_delay != -1) {
>> +        stream->priv->no_delay = !!no_delay;
>> +    }

Also, if red_socket_get_no_delay(socket) fails here
future calls to reds_stream_get_no_delay may return a value
(false?) and not fail (in case reds_stream_set_no_delay was
not called).

Uri

>> +
>>      /* deprecated fields. Filling them for backward compatibility */
>>      stream->priv->info->llen = sizeof(stream->priv->info->laddr);
>>      stream->priv->info->plen = sizeof(stream->priv->info->paddr);
>
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>