Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce file descriptor consumption #59

Merged
merged 3 commits into from
Sep 9, 2021
Merged

Reduce file descriptor consumption #59

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe708a1
Reduce file descriptor consumption
Sep 9, 2021
d7cc7a5
Fix for tvOS prior to 13
Sep 9, 2021
686ac20
Another tvOS 13 fix
Sep 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 29 additions & 25 deletions Sources/Segment/Utilities/Storage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ internal class Storage: Subscriber {
let userDefaults: UserDefaults?
static let MAXFILESIZE = 475000 // Server accepts max 500k per batch

private var fileHandle: FileHandle? = nil

init(store: Store, writeKey: String) {
self.store = store
self.writeKey = writeKey
Expand Down Expand Up @@ -241,23 +243,20 @@ extension Storage {
}

syncQueue.sync {
do {
let jsonString = event.toString()
if let jsonData = jsonString.data(using: .utf8) {
let handle = try FileHandle(forWritingTo: storeFile)
handle.seekToEndOfFile()
// prepare for the next entry
if newFile == false {
handle.write(",".data(using: .utf8)!)
}
// write the data
handle.write(jsonData)
handle.closeFile()
} else {
assert(false, "Storage: Unable to convert event to json!")
let jsonString = event.toString()
if let jsonData = jsonString.data(using: .utf8) {
fileHandle?.seekToEndOfFile()
// prepare for the next entry
if newFile == false {
fileHandle?.write(",".data(using: .utf8)!)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we absolutely know 100% of the time this explicit will work on all platforms? Makes me uneasy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would it not work on all platforms?? It's almost identical to what it was before and tests are passing hunky dory.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it just being a comma should be okay.

}
} catch {
assert(false, "Storage: failed to write event to \(storeFile), error: \(error)")
// write the data
fileHandle?.write(jsonData)
if #available(tvOS 13, *) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to add a comment why we are doing this for 13 and above but supporting 11.

try? fileHandle?.synchronize()
}
} else {
assert(false, "Storage: Unable to convert event to json!")
}
}
}
Expand All @@ -266,7 +265,8 @@ extension Storage {
syncQueue.sync {
let contents = "{ \"batch\": ["
do {
try contents.write(toFile: file.path, atomically: true, encoding: .utf8)
FileManager.default.createFile(atPath: file.path, contents: contents.data(using: .utf8))
fileHandle = try FileHandle(forWritingTo: file)
} catch {
assert(false, "Storage: failed to write \(file), error: \(error)")
}
Expand All @@ -275,23 +275,27 @@ extension Storage {

func finish(file: URL) {
syncQueue.sync {
let tempFile = file.appendingPathExtension(Storage.tempExtension)
try? FileManager.default.copyItem(at: file, to: tempFile)

let sentAt = Date().iso8601()

// write it to the existing file
let fileEnding = "],\"sentAt\":\"\(sentAt)\"}"
let endData = fileEnding.data(using: .utf8)
if let endData = endData, let handle = try? FileHandle(forWritingTo: tempFile) {
handle.seekToEndOfFile()
handle.write(endData)
handle.closeFile()
if let endData = endData {
fileHandle?.seekToEndOfFile()
fileHandle?.write(endData)
if #available(tvOS 13, *) {
try? fileHandle?.synchronize()
}
fileHandle?.closeFile()
fileHandle = nil
} else {
// something is wrong with this file, maybe delete it?
//assert(false, "Storage: event storage \(file) is messed up!")
}


let tempFile = file.appendingPathExtension(Storage.tempExtension)
try? FileManager.default.copyItem(at: file, to: tempFile)

let currentFile: Int = (userDefaults?.integer(forKey: Constants.events.rawValue) ?? 0) + 1
userDefaults?.set(currentFile, forKey: Constants.events.rawValue)
}
Expand Down