Skip to content

Conversation

@azone
Copy link
Contributor

@azone azone commented Oct 31, 2017

Fixes #791

To test:

  1. Input or paste about over 30,000 characters
  2. Scroll TextView

return textStore.attributes(at: location, effectiveRange: range)
}

private func replaceTextStoreString(_ range: NSRange, with string: String) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Considering String.replaceSubrange() is of complexity O(m) for most scenarios (where m is the sum of the length of the cached string and the modification), this method should be replaced by:

textStoreString = textStore.string

detectAttachmentRemoved(in: range)
textStore.replaceCharacters(in: range, with: str)

replaceTextStoreString(range, with: str)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should move the update logic inside of override func edited() (inside the next call in this method), and only trigger it for .editedCharacters.

That way we don't need to call both at several point in the app.

@diegoreymendez
Copy link
Contributor

@azone - Very nice find and very nice approach. Added a few comments on further improvements.

Copy link
Contributor

@diegoreymendez diegoreymendez left a comment

Choose a reason for hiding this comment

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

Aproving, since we need these changes. I'll address any changes separately.

@diegoreymendez diegoreymendez merged commit 4dae38c into wordpress-mobile:develop Nov 16, 2017
@diegoreymendez
Copy link
Contributor

Merging since Travis is stuck.

@diegoreymendez diegoreymendez changed the title Fixed large text memory issue Fixes large text memory issue. Nov 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants