Speed up search by not invoking apm

[Ingramz]

Description of the Change

This pull request implements the search capability equivalent to previously used apm search --json (--packages|--themes)? term, sorted by download count in descending order. Measuring the web request that is done inside apm, with ~118ms network latency it takes around 750-800ms to respond (according to Chrome devtools), whereas the total execution time for apm search takes around 1.5 seconds.

Searching for github and measuring using performance.now():
Before: packageManager.search() 1587ms average
After: client.search() 853ms average

Although using just apm would be cleaner, this is a significant gain in terms of perceived responsiveness. Search functionality doesn't depend on apm existing in any way and being one of the most frequently used parts of settings-view where user has to wait, it makes sense to reduce any delays where possible. Ideally there should be a library which both settings-view and apm use as that eliminates the duplication concern.

It is possible that by enabling compression at atom.io, the request response time can be further improved, making responses a fair amount faster on slower connections as huge amounts of text typically compress well.

Alternate Designs

None considered.

Benefits

Search returns faster ⚡️, doesn't feel as slow to the user.

Possible Drawbacks

A little bit more code duplication between apm and this package.

Applicable Issues

None.

[50Wliu]

It looks like install-panel is the only place that calls packageManager.search? Therefore it should be safe to 🔥 that entire method.

[Ingramz]

Absolutely. I'll get on it .

[50Wliu]

I have absolutely no clue why apm decided to alter the returned JSON from its own API to make description as a top-level property but it's really annoying (why not just change it in the API??).

Anyway, the API allows for a sort parameter so you can pass that directly in the query string rather than sorting the data afterwards. You can also get request to process the JSON for you.

Here's the version that I wrote while investigating a different issue. I think the only thing that's missing from this one is the changing of description.

    new Promise (resolve, reject) ->
      qs = {q: query}

      if options.themes
        qs.filter = 'theme'
      else if options.packages
        qs.filter = 'package'

      if options.sortBy
        qs.sort = options.sortBy

      options = {
        url: "https://atom.io/api/packages/search"
        headers: {'User-Agent': navigator.userAgent}
        qs: qs
        json: true
      }

      request options, (err, res, body) ->
        if err
          error = new Error("Searching for \u201C#{query}\u201D failed.")
          error.stderr = err.message
          reject(error)
        else
          resolve(body)

[Ingramz]

@50Wliu thank you for the feedback. I made some changes based on your version, should look nicer now.

[50Wliu]

Should just be const options = {sort: 'downloads'}. The atom.io API is documented at https://flight-manual.atom.io/atom-server-side-apis/sections/atom-package-server-api/. Though it seems like the direction parameter is a bit broken for downloads because I can't get it to sort ascending...luckily we sort descending 😀.

Also, seems weird how we sort by downloads. Now that the relevance sorting has been improved maybe we can default to relevance instead, probably in a different PR.

[Ingramz]

const options = {sort: 'downloads'}

const options = {sort: {downloads: 'desc'}}

[Ingramz]

Considering sorting was not done before, I am guessing it can be straight omitted and perhaps the reason why it works is because it has no effect (unconfirmed).

[Ingramz]

Yes, it was because of the garbage value in sort field why it seemed to work.

However, when properly sorting on serverside vs not sorting at all, we are getting way different results. The default, unsorted search seems to give more relevant results compared to what is popular, but less relevant. I will roll back the sorting part from last night.

[50Wliu]

I'm unfamiliar with proxying but is there anything special that needs to be done here to avoid proxy issues?

EDIT: Seems like some ongoing proxy work in #1010 that you could look at.

[Ingramz]

    if @httpsProxy?
      options.proxy = @httpsProxy
    if @strictSsl is false
      options.rejectUnauthorized = false

is what is needed to be added to the request options when that pull request is merged.

[50Wliu]

I don't think we are sorting by downloads right now, either. Here's what I see after I search for random and scroll down a bit.

[Ingramz]

Packages containing the search term will be moved to the top with the exact match always being the first one if there is one. This is done later in the process.

settings-view/lib/install-panel.js

Lines 221 to 223 in 9315802

	this.highlightExactMatch(this.refs.resultsContainer, query, packages)
	this.addCloseMatches(this.refs.resultsContainer, query, packages)
	this.addPackageViews(this.refs.resultsContainer, packages)

[Ingramz]

Gzip compression was enabled 🎉 . Seeing additional ~100ms improvement, but I will try rerunning the measurements from the opening post to get more accurate numbers. Currently on a moving bus with 4G internet, I am seeing sub-second or close-to-a-second overall response times.

It occurred to me that we can probably save a portion of time by reusing the TLS session from the first connection as it is quite expensive to obtain it every time via handshake. If reuse is possible using request or some other method, we could also port featured packages to use the API directly from Atom. While the user is typing, the session can be fetched via the already open/completed featured packages request and the first search could be at least one RTT faster. I would not want to roll our own http client though if there is no easy way of doing this. Update 2: not possible in current server configuration.

Update: For some reason I am not getting quite 100ms improvement on request, however I am seeing 50ms improvement (801.25ms avg). At least the website loads extremely fast now.