Review Request: Speed limit in ftp kio slave

[Thiago Macieira]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102267/#review5606
-----------------------------------------------------------



kioslave/ftp/CMakeLists.txt
<http://git.reviewboard.kde.org/r/102267/#comment4998>

    We usually do not use capitals in source code in kdelibs. (there are exceptions, but not in KIO).
    
    Also, it would be better if this were called ratecontroller.cpp, not speed controller.



kioslave/ftp/ftp.cpp
<http://git.reviewboard.kde.org/r/102267/#comment4999>

    As David said, this is a GCC extension and not portable. Please use QByteArray.



kioslave/ftp/ftp.cpp
<http://git.reviewboard.kde.org/r/102267/#comment5000>

    No arbitrary numbers in the source code. I also don't like a waiting in the FTP code. I'd rather you moved the waiting to the rate controller and found a suitably-named function.



kioslave/ftp/speedController.h
<http://git.reviewboard.kde.org/r/102267/#comment5001>

    Rename to RateController.



kioslave/ftp/speedController.h
<http://git.reviewboard.kde.org/r/102267/#comment5002>

    Suggest renaming to nextReadBlockSize().



kioslave/ftp/speedController.h
<http://git.reviewboard.kde.org/r/102267/#comment5003>

    Use QElapserTimer, not QTime.



kioslave/ftp/speedController.h
<http://git.reviewboard.kde.org/r/102267/#comment5004>

    This will not work for kio_http. You need to limit the transfer rate from the ioslave to the application, in KIO::SlaveBase::data(), or in KIO::SlaveInterface::data().
    
    You don't get access to the QTcpSocket that KIO::TCPSlaveBase uses.


- Thiago


On Aug. 9, 2011, 7:16 p.m., Tushar Mehta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102267/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2011, 7:16 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> - This patch contains the basic code which will put the limit on download speed of the ftp data transfer.
> - It is looking for "speed-limit" meta-data for deciding how much speed control is required.
> - If this meta-data is not found, code will work as it was before and no speed control related code will come into picture.
> - This patch is the most basic one which I have testing on my system and to the extent it is controlling the speed.
> - Lets say if speed limit is 30 KBps then mostly will get the avg speed around 30 to 35 KBps.
> - I am using QTime for measuring time elapsed between two socket read call and its precision is in millisecond. Looping is taking place in microsecond and thats why I am getting almost all the time 0 as time elapsed in between two calls.
> - To solve the above problem usleep is introduced to make it sync with the timer.
> 
> 
> Diffs
> -----
> 
>   kioslave/ftp/CMakeLists.txt e080b02 
>   kioslave/ftp/ftp.h 0bd375b 
>   kioslave/ftp/ftp.cpp 655524a 
>   kioslave/ftp/speedController.h PRE-CREATION 
>   kioslave/ftp/speedController.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102267/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tushar
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110810/c2390f3b/attachment.htm>