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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate synchronous I/O on main thread when displaying match results from fuzzy-native #384

Merged
merged 2 commits into from May 6, 2019

Conversation

Projects
None yet
1 participant
@nathansobo
Copy link
Contributor

commented May 2, 2019

馃崘'd with @rafeca

Description of the Change

Previously, limitations in the fuzzy-native API required us to call fs.existsSync on the main thread in order to map the relative paths in match results back into absolute paths in the presence of multiple project roots. This could block the main thread for an arbitrary amount of time and lead to performance degradation, so we decided to fix it.

The fix involved changing fuzzy-native's API to associate each match candidate with an integer identifier. Currently, we're just assigning each item's id to its index in the items array. Now, when we get a match back from fuzzy-native, we can map it back to the original item that produced the candidate via the match's id. This removes the need for synchronous I/O to disambiguate the relative path.

Alternate Designs

I'd love to replace fuzzy-native entirely, but that's out of scope right now.

Benefits

  • We no longer perform blocking I/O on the main thread, which could lead to performance problems.
  • The code for processing match results from fuzzy-native is much simpler.

Possible Drawbacks

  • Our fork of fuzzy-native has diverged even further from the upstream repository, but my understanding is that it is mostly unmaintained anyway.

Applicable Issues

#383
#382
atom/fuzzy-native#3

@nathansobo nathansobo changed the title Use new fuzzy-native API that associates match results with ids Eliminate synchronous I/O on main thread when displaying match results from fuzzy-native May 2, 2019

Use new fuzzy-native API that associates match results with ids
Co-Authored-By: Rafael Oleza <rafeca@users.noreply.github.com>

@nathansobo nathansobo force-pushed the ns-ro/remove-sync-io branch from ed21523 to 0990bf4 May 2, 2019

@nathansobo nathansobo merged commit 505b42b into master May 6, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nathansobo nathansobo deleted the ns-ro/remove-sync-io branch May 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.