groue / GRMustache.swift

Flexible Mustache templates for Swift
http://mustache.github.com/
MIT License
597 stars 155 forks source link

Parsing large templates causes tremendous memory allocation #18

Closed rmgrimm closed 8 years ago

rmgrimm commented 8 years ago

Thanks for the great library!

I've been using GRMustache.swift for several months now and have been happy with the results. I'm using v0.11.0 from CocoaPods to generate a single-page report for printing in an enterprise business application.

Recently, my team has been experiencing out of memory crashes, so I profiled the memory usage with Instruments and found the following culprit: return templateString.substringFromIndex(index).hasPrefix(string) (line 47 of TemplateParser.swift)

I have 8 total templates, listed by their size in bytes: template file sizes in bytes

My app preloads the templates in the background at start. It is protected by a GCD dispatch group, so subsequent uses of the templates wait for initial preloading to finish (which avoids a crash): appdelegate mustache preloading

As you can see in the following screenshot, Mustache allocates 96.7% of the application's total allocated memory before it is killed by iOS: mustache heap allocations with cfstring

For comparison, here is Instrument's display of the allocation statistics: overall heap allocation

The problem seems to be that Swift is creating new immutable copies of the templateString with each call to substringFromIndex(String.CharacterView.Index) -> String, and these appear to be permanent as long as the app is alive. Being that the closure atString() is called throughout Mustache's parse(_:templateId:) method, the later parts of templateString become duplicated countless times for each template, causing massive amounts of memory usage.

For my own usage, I am investigating using the sequences templateString.characters and string.characters that were introduced in iOS 9.0. This isn't ideal; I prefer to stick to an unmodified library. At the same time, I understand that compatibility with iOS 7.0+ greatly restricts string processing features in Swift.

Any help is greatly appreciated.

groue commented 8 years ago

Thanks for your detailed report, @rmgrimm. I think I'll have a little work, please hold on.

groue commented 8 years ago

If I'm not too much mistaken, both #14 and #18 should be fixed together. I'm on it.

rmgrimm commented 8 years ago

I agree, they seem to be different ways of wording the same issue. When I submitted #18, I wasn't thinking of string interning as a memory leak; sorry for the duplicate issue.

Again, thanks for your work. I look forward to seeing your solution.

groue commented 8 years ago

I'm working on a different parsing technique in https://github.com/groue/_MustacheScanner. Still a work in progress.

rmgrimm commented 8 years ago

OK. In the meantime, here's the code that I'm using as a local modification to TemplateParser.swift:

    func parse(templateString:String, templateID: TemplateID?) {
        var currentDelimiters = ParserTagDelimiters(tagDelimiterPair: tagDelimiterPair)
        let templateCharacters = templateString.characters

        let atString = { (index: String.Index, string: String?) -> Bool in
            guard let string = string else {
                return false
            }
            let endIndex = index.advancedBy(string.characters.count, limit: templateCharacters.endIndex)
            return templateCharacters[index..<endIndex].startsWith(string.characters)
        }

Again, this uses features that Xcode lists as introduced in iOS 9.0:

With this change, the memory usage of the atString() closure is greatly reduced: memory usage after changes

groue commented 8 years ago

Thanks, @rmgrimm! I'll definitely take a look at your modification!