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

Optimize title search so that cancelled binary search Promises return at the earliest possible opportunity #637

Closed
Jaifroid opened this issue Jul 9, 2020 · 2 comments · Fixed by #642
Assignees
Milestone

Comments

@Jaifroid
Copy link
Member

Jaifroid commented Jul 9, 2020

This relates to the optimization of #617. Although searches are successfully cancelled when a user starts a new search or exits the search UI, and extraneous results from a cancelled search do not display in the UI, a certain amount of binary search processing still continues from Promises which had been set up very early on in the search process. It should be possible to return a null result, or use throw, from those Promises to prevent unnecessary CPU utilization. This has to be done without simply dereferencing parameters and causing potential memory leaks. It is similar to #426, where a fair amount of processing for a previous article continues after a user has navigated away to another article. Similar techniques could be used to solve both situations.

#617 (comment) is particularly relevant to understand the issue here.

@Jaifroid Jaifroid added this to the v2.9 milestone Jul 9, 2020
@Jaifroid Jaifroid self-assigned this Jul 9, 2020
Jaifroid added a commit that referenced this issue Jul 11, 2020
Fixes #179 and #184. A complete solution to cancelling some running binary searches after user has initiated a new search will be dealt with in #637.
@Jaifroid
Copy link
Member Author

So I don't forget, here is a quick summary of what needs to be done following on from #617:

  • Move globalstate object from init.js to app.js; possibly rename appstate;
  • Pass the appstate.search object to zimArchive.js;
  • When a search is cancelled, set the appstate.search.status property to 'cancelled';
  • When a new search is undertaken, also set appstate.search.status to 'cancelled': this will be reflected in the search object passed to zimArchive.js because they both still point to the same underlying object;
  • Instantiate a new search object, and set appstate.search to this new object; the one passed to zimArchive.js will still point to the old object according to Javascript Pointer Reference craziness;
  • Insert an early cancellation at the very start of the binary search loop: if a search status is cancelled, return 0 instead of -1 or +1 -- this causes the loop to exit early;
  • Once all Promises have returned, the old search object is no longer referenced, and should be garbage collected automatically in due course.

@Jaifroid
Copy link
Member Author

Jaifroid commented Jul 12, 2020

If this works, extend the model to #426, which is a very similar case: we should be able to use the same appstate object to pass an appstate.article object, which would have a property appstate.article.status. In Service Worker mode, if we are allowed to emit a signal from the message channel that the user has clicked on a new article link, then this solution should be viable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant