From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on gnuweeb.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NO_DNS_FOR_FROM,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6 Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) by gnuweeb.org (Postfix) with ESMTPSA id 2238780961 for ; Sun, 21 Aug 2022 22:14:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gnuweeb.org; s=default; t=1661120045; bh=/vvi35Z4mOg9d7F6Hzm4mzdLgtT+VrsGS4aE5cDgioo=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=an8e2++HESEiLDELW46CAk/QWlPr0IAKZIH3zNuwNIgjOfGYzEl2ca0y/j1udwQ6M Y68EgV7GE/tfriOI9+49JExwukfdoCobEkUKUdt3v3JzPIO0vVMmBebJQFFIQSswzN n+5Cyz603ElHmn5gB8rEJTM2IagbslrUjp4JXTZOXj09h459F2kk91CUUHWiaKliEr 7BjEdNtFo7xICM1RQjLtRBcI8cvBBVbWAsYwnN4VoIGbF0vFa4QcSkXfXosDhtjjsa kY7bUFuPCgJr1v6b4G54nJiPOR+63vW/Zj0L3MuKhaOyL5GjITWzAh18bIRpCd+SXo VL1RximErpTLg== Received: by mail-lf1-f44.google.com with SMTP id m3so7237433lfg.10 for ; Sun, 21 Aug 2022 15:14:05 -0700 (PDT) X-Gm-Message-State: ACgBeo2++f7X9Bmg/JfcBQTsiMLoClGAncZpGvgEEKz6sYHFVXYQ83pa pbBwEzwNt7pGc+UNlci6MfLnMfLCy0HQFSpYkqY= X-Google-Smtp-Source: AA6agR4zJNr1Y6oWInPk0uLqA81nhHk9TUaJNLPtICRSmLlaU+IAIZinSvFnhU0q5EM41WDEWp82vx7XEcKfgEvO6ig= X-Received: by 2002:a05:6512:3501:b0:48b:205f:91a2 with SMTP id h1-20020a056512350100b0048b205f91a2mr6041876lfs.83.1661120043188; Sun, 21 Aug 2022 15:14:03 -0700 (PDT) MIME-Version: 1.0 References: <20220821112453.3026255-1-ammarfaizi2@gnuweeb.org> <20220821112453.3026255-18-ammarfaizi2@gnuweeb.org> In-Reply-To: <20220821112453.3026255-18-ammarfaizi2@gnuweeb.org> From: Alviro Iskandar Setiawan Date: Mon, 22 Aug 2022 05:13:51 +0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v1 17/22] chnet: Initial chunked request body support To: Ammar Faizi Cc: Muhammad Rizki , Kanna Scarlet , "GNU/Weeb Mailing List" Content-Type: text/plain; charset="UTF-8" List-Id: On Sun, Aug 21, 2022 at 6:24 PM Ammar Faizi wrote: > +int CHNetDataStreamChunked::ReadInternal(net::IOBuffer *buf, int read_size_arg) > +{ > + size_t read_size; > + char *ptr; > + int ret; > + > + if (unlikely(read_size_arg < 0)) > + return net::ERR_IO_PENDING; > + > + if (unlikely(is_eof_ || !read_size_arg)) > + return 0; > + > + ret = 0; > + ptr = buf->data(); > + read_size = (size_t)read_size_arg; > + > + std::unique_lock lk(buf_lock_); > + while (read_size) { > + struct net::CHNetDataStreamChunked::buf *b; > + size_t copy_size; > + > + if (unlikely(!buf_queue_.size())) { > + /* > + * At this point, the buffer is empty. > + * > + * Wait for the buffer queue to be > + * filled in. > + */ > + being_waited_.store(true, std::memory_order_release); > + buf_cond_.wait(lk, [this]{ return buf_queue_.size(); }); > + being_waited_.store(false, std::memory_order_release); > + } > + > + b = buf_queue_.front(); > + if (b->len_ == -(size_t)1UL) { > + is_eof_ = true; > + break; > + } > + > + copy_size = b->len_ - cur_pos_; > + if (copy_size > read_size) > + copy_size = read_size; > + > + memcpy(ptr, &b->buf_[cur_pos_], copy_size); > + ptr += copy_size; > + ret += copy_size; > + read_size -= copy_size; > + buf_queue_.head_++; > + } > + > + if (unlikely(is_eof_)) > + SetIsFinalChunk(); > + > + if (unlikely(!ret && !is_eof_)) > + return net::ERR_IO_PENDING; > + > + return ret; > +} I think you shouldn't return net::ERR_IO_PENDING, just sleep on the condition variable if the buffer is not ready yet, it is legal to sleep on ReadInternal(), why spend time spinning on -EAGAIN? You can end in OnReadCompleted() if you sleep on the condition variable and wait for it to get signaled: https://source.chromium.org/chromium/chromium/src/+/main:net/base/upload_data_stream.cc;l=82-87;drc=5eda14f193ef3e0131aad2f31ae172c0001b3f6d?q=upload_data_stream.cc&ss=chromium%2Fchromium%2Fsrc tq -- Viro