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 improvements #1735

Merged
merged 9 commits into from
Jun 15, 2022
Merged

Conversation

harshad1
Copy link
Collaborator

This branch has some performance improvements for regexReplace and the chunked change system.

Objective is to make a actions take < 50ms on a 100k character file with highlighting enabled (my monthly log / journal files regularly reach 70k chars). We aren't quite there, but this has brought us from 120ms to 80ms on my pixel 3

@harshad1
Copy link
Collaborator Author

harshad1 commented Jun 5, 2022

Got closed when rebasing

@harshad1 harshad1 reopened this Jun 5, 2022

final int lineEnd = StringUtils.getLineEnd(text, lineStart, selEnd);
final CharSequence line = text.subSequence(lineStart, lineEnd);
for (int i = 0; i < lineCount; i++) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Much simpler and less work being done

@@ -147,7 +146,7 @@ private void assertCorrectReplacement(String original, String expectedReplacemen

private String replaceWithFirstMatchingPattern(List<TextActions.ReplacePattern> replacePatterns, String input) {
for (TextActions.ReplacePattern replacePattern : replacePatterns) {
Matcher matcher = replacePattern.searchPattern.matcher(input);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Creating a matcher is surprisingly expensive compared to resetting it with a new input

@gsantner
Copy link
Owner

gsantner commented Jun 5, 2022

when working on the Custom templates issue I noticed that UTF-8 with bom doesn't really work on the newfile dialog - always see a empty document created then. when choosing same template with bom checked off it works.

(Not sure if realted to the file write PRs)

@harshad1
Copy link
Collaborator Author

harshad1 commented Jun 5, 2022 via email

@harshad1
Copy link
Collaborator Author

harshad1 commented Jun 5, 2022

@gsantner I can't get the tests to run any more

image

Any idea what I can do?

EDIT: Resolved. The issue was Android studio cupcake not being happy with this. I will try upgrading again after patch 2 is out. Downgraded to Bumblebee for now

@harshad1 harshad1 changed the title WIP: Performance improvements Performance improvements Jun 6, 2022
return () -> {
synchronized (sync) {
_handler.removeCallbacks(callback);
_handler.postDelayed(callback, delayMs);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a general debouncer. Similar to the Highlighting debouncer, but a simple function. I will look into simplifying the highlighting logic with this at a later date.


final char[] buf = new char[end - start];
TextUtils.getChars(source, start, end, buf, 0);
return new String(buf);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

editable.subSequence(start, end).toString() copies spans etc. This avoids that extra work

@gsantner
Copy link
Owner

gsantner commented Jun 6, 2022

EDIT: Resolved. The issue was Android studio cupcake not being happy with this.

yeah wouldn't be the first time such things happen. I only upgrade android studio and the android gradle tool when there is some absolute must

@@ -342,10 +342,16 @@ public static void showMultiChoiceDialogWithSearchFilterUI(final Activity activi
};

// Helper function to append selection count to OK button
final String okString = dopt.okButtonText != 0 ? activity.getString(dopt.okButtonText) : "";
Copy link
Collaborator Author

@harshad1 harshad1 Jun 8, 2022

Choose a reason for hiding this comment

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

Unrelated fix for when okButtonText == 0.

Text actions calls are surrounded with a try-catch all, so exceptions caused here were not detected

@harshad1
Copy link
Collaborator Author

I am satisfied with these changes

@gsantner gsantner added this to the Markor v2.10 milestone Jun 15, 2022
@gsantner gsantner merged commit e89e3f5 into gsantner:master Jun 15, 2022
@gsantner
Copy link
Owner

yes thank you, merged

@gsantner gsantner mentioned this pull request Jul 16, 2022
@harshad1 harshad1 deleted the performance_improvements branch May 10, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants