-
Notifications
You must be signed in to change notification settings - Fork 297
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Autocomplete: Enable keepalive agent for Node (#868)
We found another small way to improve overall latencies: Making sure we keep the TCP connection alive and reducing the need for subsequent SSL/TLS handshakes. In Node, reusing TCP connections is not the default behavior (like it is for example on our Go backends). To fix this, we have to make sure we create a custom network agent that is configured to keep connections allive. Since this is Node only, we are using a mutable ref objects to not run any of those code paths on the web branches. c.f. https://sourcegraph.slack.com/archives/C05497E9MDW/p1693387804620389 ## Test plan Tested using the local completion CLI script We found about a 5% latency improvement: ![image](https://github.com/sourcegraph/cody/assets/458591/d667d71a-3c04-444d-b6c8-08ea3cca70f4) <!-- Required. See https://docs.sourcegraph.com/dev/background-information/testing_principles. --> --------- Co-authored-by: Valery Bugakov <skymk1@gmail.com>
- Loading branch information
1 parent
f886c78
commit 672b649
Showing
6 changed files
with
104 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
import http from 'http' | ||
import https from 'https' | ||
|
||
import { agent } from './fetch' | ||
|
||
// The path to the exported class can be found in the npm contents | ||
// https://www.npmjs.com/package/@vscode/proxy-agent?activeTab=code | ||
const nodeModules = '_VSCODE_NODE_MODULES' | ||
const proxyAgentPath = '@vscode/proxy-agent/out/agent' | ||
const proxyAgent = 'PacProxyAgent' | ||
|
||
export function initializeNetworkAgent(): void { | ||
/** | ||
* We use keepAlive agents here to avoid excessive SSL/TLS handshakes for autocomplete requests. | ||
*/ | ||
const httpAgent = new http.Agent({ keepAlive: true, keepAliveMsecs: 60000 }) | ||
const httpsAgent = new https.Agent({ keepAlive: true, keepAliveMsecs: 60000 }) | ||
|
||
const customAgent = ({ protocol }: Pick<URL, 'protocol'>): http.Agent => { | ||
if (protocol === 'http:') { | ||
return httpAgent | ||
} | ||
return httpsAgent | ||
} | ||
|
||
agent.current = customAgent | ||
|
||
/** | ||
* This works around an issue in the default VS Code proxy agent code. When `http.proxySupport` | ||
* is set to its default value and no proxy setting is being used, the proxy library does not | ||
* properly reuse the agent set on the http(s) method and is instead always using a new agent | ||
* per request. | ||
* | ||
* To work around this, we patch the default proxy agent method and overwrite the | ||
* `originalAgent` value before invoking it for requests that want to keep their connection | ||
* alive (as indicated by the `Connection: keep-alive` header). | ||
* | ||
* c.f. https://github.com/microsoft/vscode/issues/173861 | ||
*/ | ||
try { | ||
const PacProxyAgent = (globalThis as any)?.[nodeModules]?.[proxyAgentPath]?.[proxyAgent] ?? undefined | ||
if (PacProxyAgent) { | ||
const originalConnect = PacProxyAgent.prototype.connect | ||
// Patches the implementation defined here: | ||
// https://github.com/microsoft/vscode-proxy-agent/blob/d340b9d34684da494d6ebde3bcd18490a8bbd071/src/agent.ts#L53 | ||
PacProxyAgent.prototype.connect = function (req: http.ClientRequest, opts: { protocol: string }): any { | ||
try { | ||
const connectionHeader = req.getHeader('connection') | ||
if ( | ||
connectionHeader === 'keep-alive' || | ||
(Array.isArray(connectionHeader) && connectionHeader.includes('keep-alive')) | ||
) { | ||
this.opts.originalAgent = customAgent(opts) | ||
return originalConnect.call(this, req, opts) | ||
} | ||
return originalConnect.call(this, req, opts) | ||
} catch { | ||
return originalConnect.call(this, req, opts) | ||
} | ||
} | ||
} | ||
} catch (error) { | ||
// Ignore any errors in the patching logic | ||
void error | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import type { Agent } from 'http' | ||
|
||
/** | ||
* By hard-requiring isomorphic-fetch, we ensure that even in newer Node environments that include | ||
* `fetch` by default, we still use the `node-fetch` polyfill and have access to the networking code | ||
*/ | ||
import isomorphicFetch from 'isomorphic-fetch' | ||
|
||
/** | ||
* In node environments, it might be necessary to set up a custom agent to control the network | ||
* requests being made. | ||
* | ||
* To do this, we have a mutable agent variable that can be set to an instance of `http.Agent` or | ||
* `https.Agent` (depending on the protocol of the URL) but that will be kept undefined for web | ||
* environments. | ||
* | ||
* Agent is a mutable ref so that we can override it from `fetch.node.ts` | ||
*/ | ||
export const agent: { current: ((url: URL) => Agent) | undefined } = { current: undefined } | ||
|
||
export function fetch(input: RequestInfo | URL, init?: RequestInit): Promise<Response> { | ||
return isomorphicFetch(input, { | ||
...init, | ||
agent: agent.current, | ||
} as RequestInit) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters