patternswiftMinor
Writing a chunk of a file to a socket in Swift
Viewed 0 times
swiftchunkfilewritingsocket
Problem
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?
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 0 else {return Int64(read)}
var writeCounter = 0
while writeCounter 0 else {return Int64(writeResult)}
writeCounter = writeCounter + writeResult
writed += UInt64(writeCounter)
}
}
}Solution
The type annotations in
are not necessary, this can be simplified to
The variable names can be improved:
"written", not "writed", a better variable name might be
bytes written to the target file.
The conversion to
can overflow on a 32-bit architecture. If the number of bytes to
read is computed as
instead then it can not overflow and only one call to
is needed.
There is a bug in
which should be
otherwise the number of bytes written is added multiple times to
the total counter if multiple
(and then
With the outer loop
avoided by changing the outer loop to
Then also the logic becomes clearer and the condition in
can be simplified.
Your function can only return
therefore the return type
One possibility would be to change the return type to
success or failure. Another possibility is to return the total number
of bytes copied in the success case (which can be less than the
parameter if end-of-file was encountered), or
Yet another possibility is to
details on the problem to the caller.
I would not use empty external parameter names (
is more descriptive than
The
Putting it all together the function could look like this:
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.
let bufferSize:Int = 1024
var read:Int = 0are not necessary, this can be simplified to
let bufferSize = 1024
var read = 0The variable names can be improved:
read is not ideal since there is a system callread() 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 ofbytes written to the target file.
The conversion to
Int inlet 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 beavoided 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 indicatesuccess or failure. Another possibility is to return the total number
of bytes copied in the success case (which can be less than the
countparameter if end-of-file was encountered), or
nil on failure.Yet another possibility is to
throw an error, that allows to reportdetails on the problem to the caller.
I would not use empty external parameter names (
_), a callsendChunk(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 0 else {
return bytesRead == 0 ? totalBytesWritten : nil
}
var bytesWritten = 0
while bytesWritten 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.
Code Snippets
let bufferSize:Int = 1024
var read:Int = 0let bufferSize = 1024
var read = 0let remaining = Int(count - writed)let readAmount = Int(min(count - totalBytesWritten, Int64(bufferSize)))writed += UInt64(writeCounter)Context
StackExchange Code Review Q#160992, answer score: 4
Revisions (0)
No revisions yet.