Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Ensure completion to respond faster #1209

Merged
merged 6 commits into from
Sep 14, 2017
Merged

Ensure completion to respond faster #1209

merged 6 commits into from
Sep 14, 2017

Conversation

uudashr
Copy link
Contributor

@uudashr uudashr commented Sep 12, 2017

I put some guard for 1sec timeout for getImportablePackages()

@uudashr uudashr closed this Sep 12, 2017
@uudashr uudashr reopened this Sep 12, 2017
@uudashr uudashr closed this Sep 13, 2017
@uudashr uudashr reopened this Sep 13, 2017
@uudashr
Copy link
Contributor Author

uudashr commented Sep 13, 2017

@ramya-rao-a please check

src/goSuggest.ts Outdated
importablePkgsPromise = Promise.race([timeout(1000).then(() => this.pkgsList), importablePkgsPromise]);
}

let setPkgsList = importablePkgsPromise.then(pkgMap => {
this.pkgsList = pkgMap;
Copy link
Contributor

Choose a reason for hiding this comment

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

How about this?

let importablePkgsPromise = getImportablePackages(vscode.window.activeTextEditor.document.fileName).then(pkgMap => this.pkgList = pkgMap);

let setpkgslist = Promise.race([timeout(1000).then(() => this.pkgsList), importablePkgsPromise]);

Case when gopkgs runs under a second:
Completion will always use the latest result

Case when gopkgs runs over a second:
Completion will use cache. If its the first completion call, then no pkgs will be available which is fine, we can attribute this to cold start
When gopkgs eventually finishes, it will update the result, so that the next completion gets to use a more recent cache

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason I am suggesting the above is that with your current design

  • a user can still get stuck waiting for the very first call to completion
  • a user can get stale data if gopkgs always takes over 1 second, and the next call from completion happens after 5 sec (the cache expiry time)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh... I see

@uudashr uudashr closed this Sep 13, 2017
@uudashr uudashr reopened this Sep 13, 2017
@ramya-rao-a
Copy link
Contributor

@uudashr The Test listPackages test timesout and so fails for the tip consistently in travis ( I re-ran a couple of times).

I dont have Go tip, you have any ideas about this?

@uudashr uudashr closed this Sep 14, 2017
@uudashr uudashr reopened this Sep 14, 2017
@uudashr
Copy link
Contributor Author

uudashr commented Sep 14, 2017

@ramya-rao-a Tip version on Mac looks good on my machine. Re-run the travis build and all good.

@ramya-rao-a ramya-rao-a merged commit 9618aa0 into microsoft:master Sep 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants