fuel is ~22x slower than java.net.http.HttpClient

[jsyeo]

Fuel seems to be very slow when sending a sending a request in its body. This is even more pronounced when the payload gets bigger (> 1MB). When benchmarked against the java.net.http.HttpClient, I noticed that fuel is 24 times slower than the http client that came with Java 11. This is the code I used to do a simple benchmark.

import com.github.kittinunf.fuel.httpPost
import java.net.URI
import java.net.http.HttpClient
import java.net.http.HttpRequest
import java.net.http.HttpResponse
import java.nio.file.Files
import java.nio.file.Paths

fun main() {
    val payload = Files.readString(Paths.get("large.json"))
    val url = "http://localhost:4567/lol"

    // fuel client
    val t = dumbBenchmark {
      url.httpPost().body(payload).response()
    }
    println("fuel client took $t ms on average")

    // java.net.http.HttpClient
    val client = HttpClient.newHttpClient()
    val req = HttpRequest.newBuilder()
        .uri(URI(url))
        .POST(HttpRequest.BodyPublishers.ofString(payload))
        .setHeader("Content-Type", "application/json")
        .build()

    val t2 = dumbBenchmark {
        client.send(req, HttpResponse.BodyHandlers.ofString())
    }
    println("httpclient took $t2 ms on average")
}

// Runs the lambda five times, time each invocation, and return the average  
fun dumbBenchmark(f: () -> Unit): Double = (1..5).map {
    val start = System.currentTimeMillis()
    f.invoke()
    val end = System.currentTimeMillis()
    end - start
}.average()

Here I am running a local sinatra server at localhost:4567, the server simply returns 200 OK for any kind of input. When I send a payload of ~2.2MB I get these results:

fuel client took 7488.2 ms on average
httpclient took 330.2 ms on average

I suspect this is due to the bufferring of the ProgressOutputStream in HttpClient.kt. I find it odd that even on a request that do not need progress reporting, fuel would still buffer the output stream. We should probably not buffer when a progress callback is not given to the request.

[SleeplessByte]

What are the results of https://github.com/jsyeo/fuel/commit/04d6b25f44fe356866998352a6e7e982e86f281d?

[jsyeo]

fuel client took 528.2 ms on average
httpclient took 427.8 ms on average

Here you go!

[tfactor2]

@jsyeo I've experienced similar issue on v.2.2.0 but didn't notice this bug report - therefore did my own investigation.
The original problem stated in description will not be solved by the fix - it's a workaround - if I still need a progress callback the performance drop will be huge.

The problem is located in the ProgressOutputStream that is inherited from java.io.FilterOutputStream. The FilterOutputStream has very inefficient implementation of write(byte[] b, int off, int len) that instead of calling the wrapped OutputStream write(byte[] b, int off, int len) calls write(int b) multiple times (actually number of bytes of the body, for 1M file it will be 1M times!).
SocketOutputStream used by HttpUrlConnection in turn supports batch writing.

So the fix could be replacing the call of super.write(b, off, len) with out.write(b, off, len) in ProgressOutputStream:

class ProgressOutputStream(stream: OutputStream, val onProgress: WriteProgress) : FilterOutputStream(stream) {
    override fun write(b: ByteArray?, off: Int, len: Int) {
        out.write(b, off, len) // 'out' instead of 'super'
...

@SleeplessByte , wdyt?

[SleeplessByte]

When writing this I came across it but I completely forgot about this
(it says something about inefficient implementations of write)

Yes, if that fix passes the tests I would love it .

On 2019-08-13 17:42, tfactor2 wrote:

@jsyeo [1] I've experienced similar issue on v.2.2.0 but didn't notice
this bug report - therefore did my own investigation.
The original problem stated in description will not be solved by the
fix - it's a workaround - if I still need a progress callback the
performance drop will be huge.

The problem is located in the ProgressOutputStream that is inherited
from java.io.FilterOutputStream. The FilterOutputStream has very
inefficient implementation of write(byte[] b, int off, int len) that
instead of calling the wrapped OutputStream write(byte[] b, int off,
int len) calls write(int b) multiple times (actually number of bytes
of the body, for 1M file it will be 1M times!).
SocketOutputStream used by HttpUrlConnection in turn supports batch
writing.

So the fix could be replacing the call of super.write(b, off, len)
with out.write(b, off, len) in ProgressOutputStream:

class ProgressOutputStream(stream: OutputStream, val onProgress:
WriteProgress) : FilterOutputStream(stream) {
override fun write(b: ByteArray?, off: Int, len: Int) {
out.write(b, off, len) // 'out' instead of 'super'
...

@SleeplessByte [2] , wdyt?

--
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub [3], or mute the
thread [4].

Links:

[1] https://github.com/jsyeo
[2] https://github.com/SleeplessByte
[3]
https://github.com/kittinunf/fuel/issues/667?email_source=notifications&email_token=AAO7SWAVO2263WP2H7XAPETQELI5NA5CNFSM4IJUWDOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4GCMXA#issuecomment-520889948
[4]
https://github.com/notifications/unsubscribe-auth/AAO7SWHD3HGAWWKNWBFKVHTQELI5NANCNFSM4IJUWDOA

[tfactor2]

could it be done (pull request created) in scope of this issue?