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

Follow up: Investigate dynamic sized search string array buffer for FastSearch #51151

Closed
hannojg opened this issue Oct 21, 2024 · 4 comments
Closed

Comments

@hannojg
Copy link
Contributor

hannojg commented Oct 21, 2024

This is a follow up from this PR:

which was created for this issue ticket originally:

This is a follow up task.

Currently in FastSearch we create an array buffer which represents large string of all search values. We hard coded a length of 400k:

function createFastSearch<T>(dataSets: Array<SearchableData<T>>) {
Timing.start(CONST.TIMING.SEARCH_CONVERT_SEARCH_VALUES);
const maxNumericListSize = 400_000;
// The user might provide multiple data sets, but internally, the search values will be stored in this one list:
let concatenatedNumericList = new Uint8Array(maxNumericListSize);

This has a few drawbacks:

  • It can take too much memory (and instantiation time) to create the array if the search string won't be that long
  • If the search string is very long (over 400k characters, which is an unlikely use-case, but still) we would miss any search value that comes after the 400k chars

We want to investigate how this can be improved.

@hannojg
Copy link
Contributor Author

hannojg commented Dec 30, 2024

@melvin-bot melvin-bot bot added the Overdue label Dec 30, 2024
@melvin-bot melvin-bot bot closed this as completed Jan 6, 2025
Copy link

melvin-bot bot commented Jan 6, 2025

@hannojg, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

@hannojg
Copy link
Contributor Author

hannojg commented Jan 7, 2025

@mountiny can you please reopen? The initial PR for changing the search algorithm to a search tree took longer than anticipated. We are going to look into these items next, so its still l a priority!

@mountiny mountiny reopened this Jan 7, 2025
@hannojg
Copy link
Contributor Author

hannojg commented Jan 16, 2025

Whops sorry, I confused the ticket. This one is actually done, the PR for this has been merged !

@hannojg hannojg closed this as completed Jan 16, 2025
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

2 participants