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

Optimize search start and search end index computation while finding … #134

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Optimize search start and search end index computation while finding … #134

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 19 additions & 15 deletions spyglass/src/main/java/com/linkedin/android/spyglass/tokenization/impl/WordTokenizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,12 +376,23 @@ protected int getSearchStartIndex(final @NonNull Spanned text, int cursor) {
}

// Get index of the end of the last span before the cursor (or 0 if does not exist)
MentionSpan[] spans = text.getSpans(0, text.length(), MentionSpan.class);
int closestToCursor = 0;
for (MentionSpan span : spans) {
int end = text.getSpanEnd(span);
if (end > closestToCursor && end <= cursor) {
closestToCursor = end;
int closestLastSpanEnd = 0;
// iterate over all spans before the cursor
// we do this by finding the next span transition and looking back to find the closest span to the cursor
int nextSpanStart;
for (int searchStartIndex = 0; searchStartIndex < cursor; searchStartIndex = nextSpanStart) {

// find the next span transition
nextSpanStart = text.nextSpanTransition(searchStartIndex, text.length(), MentionSpan.class);

// get the spans from searchStartIndex to nextSpanStart
// of these, we find the closest span to the cursor
MentionSpan[] closestLastSpans = text.getSpans(searchStartIndex, nextSpanStart, MentionSpan.class);
for (MentionSpan span : closestLastSpans) {
int end = text.getSpanEnd(span);
if (end > closestLastSpanEnd && end <= cursor) {
closestLastSpanEnd = end;
}
}
Comment on lines +380 to 396
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure that this is actually faster given that the new code has more calls to get the spans for different substrings and to compute next span transitions (which internally will loop through all the spans, e.g. see this).

It is also worth noting that the new code is a fair bit longer/more complicated.

I'd like to understand the context for changing this. What is the reason for these changes? Are you running into measurable performance issues in your app? Have you benchmarked these methods? More info would be helpful. If we're going to make the code more complicated, we need to understand what we're getting in return.

}

Expand All @@ -393,7 +404,7 @@ protected int getSearchStartIndex(final @NonNull Spanned text, int cursor) {
}

// Return whichever is closer before to the cursor
return Math.max(closestToCursor, lineStartIndex);
return Math.max(closestLastSpanEnd, lineStartIndex);
}

/**
Expand All @@ -411,14 +422,7 @@ protected int getSearchEndIndex(final @NonNull Spanned text, int cursor) {
}

// Get index of the start of the first span after the cursor (or text.length() if does not exist)
MentionSpan[] spans = text.getSpans(0, text.length(), MentionSpan.class);
int closestAfterCursor = text.length();
for (MentionSpan span : spans) {
int start = text.getSpanStart(span);
if (start < closestAfterCursor && start >= cursor) {
closestAfterCursor = start;
}
}
Comment on lines -414 to -421
Copy link
Contributor

Choose a reason for hiding this comment

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

This is nifty, I didn't know about the nextSpanTransition(..) method when I wrote this many years ago, very nice! 👍

int closestAfterCursor = text.nextSpanTransition(cursor, text.length(), MentionSpan.class);

// Get the index of the end of the line
String textString = text.toString().substring(cursor, text.length());
Expand Down
Loading