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.

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

Performance of PurchasesReceiptParser.default.parse(from:) could be drastically improved. #4006

Open
Pikacz opened this issue Jun 28, 2024 · 3 comments
Assignees

Comments

@Pikacz
Copy link

Pikacz commented Jun 28, 2024

Hey guys! I played a bit with profiler and was able to make PurchasesReceiptParser.default.parse(from:) way faster.

Here are results that I've observed on iPhone 8.

Fastest before changes Slowest after changes Improvement
Time measured via Date 0.0146 0.0035 4 times faster
Cycle measured via mach_absolute_time 352095 85862 4 times faster
Profiler counters Cycles 36 953 516 6 863 165 5 times faster
Profiler counters INST_ALL 76 994 013 7 666 052 10 times less
Profiler counters L1D_TLB_MISS 89 366 17 531 5 times less

Unfortunately I don't have active developer account and I only tested on small receipts that I've found over internet. If you have any big receipt please send me a base64String.

Surprisingly reason of slowness is Date parsing. In particular ISO8601DateFormatter.default called from ArraySlice<UInt8>.toDate. Since all receipts I see have date format yyyy-MM-dd'T'HH:mm:ssZ it's easy to parse components manually. Here is the whole change:

    func toDate() -> Date? {
        if let fastParsed = toDateTryFastParsing() {
            // This approach is around ~60% faster than `ISO8601DateFormatter.default`
            return fastParsed
        }
        guard let dateString = String(bytes: Array(self), encoding: .ascii) else { return nil }

        return ISO8601DateFormatter.default.date(from: dateString)
    }

    func toData() -> Data {
        return Data(self)
    }

}

private extension ArraySlice where Element == UInt8 {
    static let toDateCalendar: Calendar = {
        var calendar = Calendar(identifier: .gregorian)
        calendar.timeZone = TimeZone(secondsFromGMT: 0)!
        return calendar
    }()

    private func toDateTryFastParsing() -> Date? {
        // expected format 2015-08-10T07:19:32Z
        guard count == 20 else { return nil }
        let asciiZero: UInt8 = 48
        let asciiNine: UInt8 = 57
        let asciiDash: UInt8 = 45
        let asciiColon: UInt8 = 58
        let asciiT: UInt8 = 84
        let asciiZ: UInt8 = 90
        let limits: [(min: UInt8, max: UInt8)] = [
            (asciiZero, asciiNine), (asciiZero, asciiNine), (asciiZero, asciiNine), (asciiZero, asciiNine), // year
            (asciiDash, asciiDash),
            (asciiZero, asciiNine), (asciiZero, asciiNine), // month
            (asciiDash, asciiDash),
            (asciiZero, asciiNine), (asciiZero, asciiNine), // day
            (asciiT, asciiT),
            (asciiZero, asciiNine), (asciiZero, asciiNine), // hour
            (asciiColon, asciiColon),
            (asciiZero, asciiNine), (asciiZero, asciiNine), // minute
            (asciiColon, asciiColon),
            (asciiZero, asciiNine), (asciiZero, asciiNine), // second
            (asciiZ, asciiZ)
        ]
        for (character, limit) in zip(self, limits) {
            guard limit.min <= character,
                  character <= limit.max else { return nil }
        }

        let year = toDateParseAsciiNumber(from: 0, to: 4)
        let month = toDateParseAsciiNumber(from: 5, to: 7)
        guard 1 <= month,
              month <= 12 else { return nil }
        let day = toDateParseAsciiNumber(from: 8, to: 10)
        guard 1 <= day,
              day <= 31 else { return nil }
        let hour = toDateParseAsciiNumber(from: 11, to: 13)
        guard 0 <= hour,
              hour <= 23 else { return nil }
        let minute = toDateParseAsciiNumber(from: 14, to: 16)
        guard 0 <= minute,
              minute <= 59 else { return nil }
        let second = toDateParseAsciiNumber(from: 17, to: 19)
        guard 0 <= second,
              second <= 59 else { return nil }

        let components = DateComponents(
            year: year, month: month, day: day, hour: hour, minute: minute, second: second
        )
        return Self.toDateCalendar.date(from: components)
    }

    private func toDateParseAsciiNumber(from: Int, to: Int) -> Int { // swiftlint:disable:this identifier_name
        let asciiZero: UInt8 = 48
        var index = from + startIndex
        let end = to + startIndex
        var result = 0
        while index < end {
            let digit = self[index] - asciiZero
            result = result * 10 + Int(digit)
            index += 1
        }
        return result
    }
}

I have prepared branch with tests ensuring that this change has no impact on SDK behavior. Unfortunately I don't have permission to push. Could you grant me permissions so I could prepare pull-request and you could decide if you want this improvement or not?

Btw it's crazy that this change has so huge impact. I would imagine that Self.toDateCalendar.date(from: components) does most of the work. It also shows that industry standards are really bad. Apple's server had some kind integer counting seconds, they wasted compute to create string, on device we receive string and waste compute to calculate some kind integer counting seconds 🙃

@RCGitBot
Copy link
Contributor

👀 We've just linked this issue to our internal tracker and notified the team. Thank you for reporting, we're checking this out!

@aboedo
Copy link
Member

aboedo commented Sep 24, 2024

@Pikacz I'm very sorry that we missed this issue!! 🤦‍♂️

Thanks for posting!

I think this makes sense. The best process to open up a PR is to fork the repo, then create a PR from your fork to our repo. Let me know if you need a hand with it!
And apologies again for the very late response!

@aboedo aboedo self-assigned this Sep 24, 2024
@Pikacz
Copy link
Author

Pikacz commented Sep 24, 2024

Hey Andy!

I'm never really worked with GitHub. Please let me know if I set pull request correctly
#4297

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants