4
\$\begingroup\$

I'm writing a chunk of a file from an offset to a socket.

This is my implementation and it's working OK from my tests.

Any comments or speed improvements?

private func sendChunk(_ source: Int32, _ target: Int32, _ offset: off_t, _ count: UInt64) -> Int64 {
    let bufferSize:Int = 1024
    var buffer = [UInt8](repeating: 0, count: bufferSize)
    var writed:UInt64 = 0
    var read:Int = 0
    while true {
        let remaining = Int(count - writed)
        if remaining < bufferSize {
            read = pread(source, &buffer, remaining, offset + Int64(writed))
        } else {
            read = pread(source, &buffer, buffer.count, offset + Int64(writed))
        }
        guard read > 0 else {return Int64(read)}
        var writeCounter = 0
        while writeCounter < read, writed < count {
        let writeResult = write(target, &buffer + writeCounter, read - writeCounter)
            guard writeResult > 0 else {return Int64(writeResult)}
            writeCounter = writeCounter + writeResult
            writed += UInt64(writeCounter)
        }
    }
}
\$\endgroup\$
4
\$\begingroup\$

The type annotations in

let bufferSize:Int = 1024
var read:Int = 0

are not necessary, this can be simplified to

let bufferSize = 1024
var read = 0

The variable names can be improved: read is not ideal since there is a system call read() with the same name. The past participle of "to write" is "written", not "writed", a better variable name might be totalBytesWritten since it contains the accumulated number of bytes written to the target file.

The conversion to Int in

    let remaining = Int(count - writed)

can overflow on a 32-bit architecture. If the number of bytes to read is computed as

    let readAmount = Int(min(count - totalBytesWritten, Int64(bufferSize)))

instead then it can not overflow and only one call to pread() is needed.

There is a bug in

        writed += UInt64(writeCounter)

which should be

        writed += UInt64(writeResult)

otherwise the number of bytes written is added multiple times to the total counter if multiple write() calls were necessary (and then count - writed can crash).

With the outer loop

while true { ... }

pread() is called once more if all data has been copied. That can be avoided by changing the outer loop to

while totalBytesWritten < count { ... }

Then also the logic becomes clearer and the condition in

    while writeCounter < read, writed < count { ... }

can be simplified.

Your function can only return 0 (on success) or -1 (on error), therefore the return type Int64 makes no sense.

One possibility would be to change the return type to Bool to indicate success or failure. Another possibility is to return the total number of bytes copied in the success case (which can be less than the count parameter if end-of-file was encountered), or nil on failure. Yet another possibility is to throw an error, that allows to report details on the problem to the caller.

I would not use empty external parameter names (_), a call

sendChunk(source: sfd, target: dfd, offset: 2, count: 5)

is more descriptive than

sendChunk(sfd, dfd, 2, 5)

The offset parameter could be defined with an default value zero.

Putting it all together the function could look like this:

func sendChunk(source: Int32, target: Int32, offset: off_t = 0, count: UInt64) -> UInt64? {

    let bufferSize = 1024
    var buffer = [UInt8](repeating: 0, count: bufferSize)

    var totalBytesWritten: UInt64 = 0
    while totalBytesWritten < count {
        let readAmount = Int(min(count - totalBytesWritten, UInt64(bufferSize)))
        let bytesRead = pread(source, &buffer, readAmount, offset + off_t(totalBytesWritten))
        guard bytesRead > 0 else {
            return bytesRead == 0 ? totalBytesWritten : nil
        }

        var bytesWritten = 0
        while bytesWritten < bytesRead {
            let amount = write(target, &buffer + bytesWritten, bytesRead - bytesWritten)
            guard amount > 0 else {
                return nil
            }
            bytesWritten += amount
        }
        totalBytesWritten += UInt64(bytesWritten)
    }

    return totalBytesWritten
}

Possible speed improvements: Most time will be spent in the read/write system calls. One thing that you can do is to increase the buffer size.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.