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

Multiple fetch requests are slow. #3192

Closed
danielschwartz85 opened this issue Jan 23, 2019 · 11 comments · Fixed by #3531
Closed

Multiple fetch requests are slow. #3192

danielschwartz85 opened this issue Jan 23, 2019 · 11 comments · Fixed by #3531
Assignees
Labels

Comments

@danielschwartz85
Copy link

danielschwartz85 commented Jan 23, 2019

Hi,

As a plugin (Applitools) we need to use the browser to download many resources, apx 70 per page.
The download speed seems very slow compared to a regular browser.

  • My reproduction gave resulted of 5.5 seconds in Cypress vs 300ms in regular browser

Current behavior:

Fetch requests seem very slow for fetching many resources (compared to regular browser).

(the fetch "slowness" is exhibited in all frames)

Desired behavior:

A faster fetch method (maybe one that doesn't go through the proxy ?).

Steps to reproduce: (app code and test code)

See my reproduction:
https://github.com/danielschwartz85/cypress-proxy-speed
(This is a Cypress app with a test that shows the slow download speeds)

It uses this page:
https://danielschwartz85.github.io/cypress-fetch-page/

Versions

Cypress 3.1.4
Ubuntu 18.04.01
Electron 59, Chrome 71

Tnx!

@jennifer-shehane jennifer-shehane added type: performance 🏃‍♀️ Performance related stage: ready for work The issue is reproducible and in scope labels Jan 23, 2019
@jennifer-shehane
Copy link
Member

Thank you so much for the detailed issue! I was indeed able to reproduce this. We'll have the team look into what could be causing this issue.

Local run outside of Cypress
screen shot 2019-01-23 at 4 46 06 pm

Visit in Cypress (even in new tab) - it's noticeably slower
screen shot 2019-01-23 at 4 47 19 pm

@danielschwartz85
Copy link
Author

Hi, Any updates on this issue ?
Tnx.

@jennifer-shehane
Copy link
Member

No updates. We will comment when work is done on this issue.

@cypress-bot cypress-bot bot added stage: backlog and removed stage: ready for work The issue is reproducible and in scope labels Feb 7, 2019
@flotwig
Copy link
Contributor

flotwig commented Mar 4, 2019

The issue seems to be related to the https-proxy. I ran the cypress-proxy-speed test suite against the cypress-slow-fetch website, hosted on HTTP at http://cypress-slow-fetch-http.chary.us/, and the loading time seems to be pretty normal once the assets are coming over HTTP:

peek 2019-03-04 17-22

I'm thinking that we're not properly re-using the TCP sockets made when we do intercepted HTTPS connections. I'll keep looking in to it.

Spec code:

describe('slow get', () => {
    it('http get it', () => {
        let url = 'http://cypress-slow-fetch-http.chary.us/'
        cy.visit(url);
    });
});

@flotwig
Copy link
Contributor

flotwig commented Mar 14, 2019

#3531, when released, will add a couple of enhancements that speed up the proxy layer:

  1. Switched to using a custom HTTP Agent implementation for all requests. Our implementation uses keep-alive for all HTTP/HTTPS requests, even if they go through a proxy. This enables us to achieve performance very close to the browser's performance.

  2. Disabled Nagle's algorithm on the socket between Cypress and Chrome when going through the https-proxy. This greatly improves the performance of HTTPS requests since it means we don't unnecessarily buffer the proxied conversation.

Originally, this test case took 1705ms on my system. After these changes, your test case runs in 660ms on my system.

That's compared to 336ms in Chromium with HTTP/2 disabled, and 126ms in Chromium with HTTP/2 enabled.

There's still more we could do to improve performance:

  1. Add HTTP/2 support - the main reason Cypress still isn't as fast as the browser, even after those changes, is because we don't take advantage of HTTP/2 where available. GitHub Pages supports HTTP/2 and that protocol is giving you those blazing fast load times you're seeing. Created an issue for it at Add HTTP/2 Support in Proxy Layer #3708

  2. Cypress actually creates its own HTTPS server in https-proxy to serve HTTPS requests. If we didn't bother creating a TCP socket to make those requests to the internal HTTPS server, that would improve throughput. Issue: Don't create a new socket to do HTTPS interception through SNI server #3833

@bahmutov
Copy link
Contributor

bahmutov commented Mar 14, 2019 via email

@flotwig
Copy link
Contributor

flotwig commented Mar 14, 2019

Gotta credit @brian-mann for this one, he eagle-eyed the setNoDelay option in the Node.js docs that we were previously unaware of

@bahmutov
Copy link
Contributor

bahmutov commented Mar 14, 2019 via email

@flotwig flotwig mentioned this issue Mar 27, 2019
38 tasks
brian-mann added a commit that referenced this issue Apr 1, 2019
* https-proxy: unused file

* server: wrap all https requests that use a proxy

* server: use request lib in ensureUrl if proxy is in use. this makes runs tab work behind a proxy

* electron: pass --proxy-server to app itself, so the embedded github login page works

* cli: first attempt at env vars from windows registry

* cli: api cleanup

* cli: lint

* cli: fix crash on no proxy, add tests

* add desktop-gui watch to terminals.json

* cli: pass along --proxy-source

* electron: pass --proxy-bypass-list too

* server: whitelist proxy* args

* cli: better wording

* desktop-gui: display proxy settings

* extension: force proxy [wip]

* extension: finally, i am victorious over coffeescript

* extension: add -loopback to bypasslist

* extension: revert changes

Revert "extension: force proxy [wip]"

This reverts commit 3ab6ba4.

* desktop-gui: skip proxysettings if there aren't any

* https-proxy, server: proxy directConnections using https-proxy-agent

* https-agent: pool httpsAgents

* https-proxy: work when they're not on a proxy

* https-proxy: ci - use agent 1.0

* https-proxy: tests

* desktop-gui: hide proxy settings when not using proxy

* https-proxy: pass req through to https-proxy-agent callback

* cli: use get-windows-proxy

* desktop-gui: always show proxy settings

* server: use get-windows-proxy

* electron, server: supply electron proxy config when window launched

* server: fix

* https-proxy: cleanup

* server: clean up ensureUrl

* https-proxy: cleanup

* cli: fix

* cli: fix destructuring

* server: enable ForeverAgent to pool HTTPS/HTTP connections

#3192

* server: updating snapshot

* https-proxy: don't crash, do error if proxy unreachable

* https-proxy:

* get-windows-proxy@1.0.0

* https-proxy: use proxy-from-env to decide on a proxy for a url

* server: fallback to HTTP_PROXY globally if HTTPS_PROXY not set

* server: proxy args test

* cli: add proxy tests

* cli: add test that loadSystemProxySettings is called during download

* cli, server: account for the fact that CI has some proxy vars set

* https-proxy: ""

* cli, https-proxy, server: ""

* desktop-gui: update settings gui

* desktop-gui: cypress tests for proxy settings

* server: strict undefined check

* cli, server: move get-windows-proxy to scope, optionalDeps

* server, cli: use new and improved get-windows-proxy

* cli, server: 1.5.0

* server: re-check for proxy since cli may have failed to load the lib

* server, cli: 1.5.1

* server: NO_PROXY=localhost by default, clean up

* https-proxy: disable Nagle's on proxy sockets

\#3192

* https-proxy: use setNoDelay on upstream, cache https agent

* https-proxy: test basic auth

* https-proxy: add todo: remove this

* server: add custom HTTP(s) Agent implementation w keepalive, tunneling

* server: typescript for agent

* add ts to zunder

* server: more ts

* ts: add missing Agent type declaration

* server: create CombinedAgent

* server: use agent in more places

* ts: more declarations

* server: make script work even if debug port not supplied

* server: begin some testing

* server, ts: agent, tests

* server: test

* server: agent works with websockets now

* server: update snapshot

* server: work out some more bugs with websockets

* server: more websockets

* server: add net_profiler

* https-proxy: fix dangling socket on direct connection

* server: fix potential 'headers have already been sent'

* https-proxy: nab another dangler

* server: update test to expect agent

* https-proxy: fix failing test

* desktop-gui: change on-link

* server: add tests for empty response case

* server: tests

* server: send keep-alive with requests

* server: make net profiler hook on socket.connect

* server: only hook profiler once

* server: update tests, add keep-alive test

* server: only regen headers if needed

* server: move http_overrides into CombinedAgent, make it proxy-proof

for #112

* server: update snapshot

* server: undo

* server: avoid circular dependency

* https-proxy, server: use our Agent instead of https-proxy-agent

* server: add dependency back

* cli: actually use proxy for download

* server, launcher, ts: typescript

* Revert "server, launcher, ts: typescript"

This reverts commit d3f8b8b.

* Revert "Revert "server, launcher, ts: typescript""

This reverts commit 818dfdf.

* ts, server: respond to PR

* server, ts: types

* ts: really fix types

* https-proxy, server: export CA from https-proxy

* agent, server, https-proxy: move agent to own package

* agent => networking, move connect into networking

* fix tests

* fix test

* networking: respond to PR changes, add more unit tests

* rename ctx

* networking, ts: add more tests

* server: add ensureUrl tests

* https-proxy: remove https-proxy-agent

* server: use CombinedAgent for API

* server: updates

* add proxy performance tests

* add perf tests to workflow

* circle

* run perf tests with --no-sandbox

* networking, ts: ch-ch-ch-ch-changes

* server, networking: pr changes

* run networking tests in circle

* server: fix performance test

* https-proxy: test that sockets are being closed

* https-proxy: write, not emit

* networking: fix test

* networking: bubble err in connect

* networking: style

* networking: clean p connect error handling

* networking => network

* server: make perf tests really work

* server: really report

* server: use args from browser

* server: use AI to determine max run time

* server: load electron only when needed


Co-authored-by: Brian Mann <brian@cypress.io>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Apr 1, 2019

The code for this is done in cypress-io/cypress#3531, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@danielschwartz85
Copy link
Author

Thanks for the work ! 👍

laurinenas pushed a commit to laurinenas/cypress that referenced this issue Apr 28, 2019
* https-proxy: unused file

* server: wrap all https requests that use a proxy

* server: use request lib in ensureUrl if proxy is in use. this makes runs tab work behind a proxy

* electron: pass --proxy-server to app itself, so the embedded github login page works

* cli: first attempt at env vars from windows registry

* cli: api cleanup

* cli: lint

* cli: fix crash on no proxy, add tests

* add desktop-gui watch to terminals.json

* cli: pass along --proxy-source

* electron: pass --proxy-bypass-list too

* server: whitelist proxy* args

* cli: better wording

* desktop-gui: display proxy settings

* extension: force proxy [wip]

* extension: finally, i am victorious over coffeescript

* extension: add -loopback to bypasslist

* extension: revert changes

Revert "extension: force proxy [wip]"

This reverts commit 3ab6ba4.

* desktop-gui: skip proxysettings if there aren't any

* https-proxy, server: proxy directConnections using https-proxy-agent

* https-agent: pool httpsAgents

* https-proxy: work when they're not on a proxy

* https-proxy: ci - use agent 1.0

* https-proxy: tests

* desktop-gui: hide proxy settings when not using proxy

* https-proxy: pass req through to https-proxy-agent callback

* cli: use get-windows-proxy

* desktop-gui: always show proxy settings

* server: use get-windows-proxy

* electron, server: supply electron proxy config when window launched

* server: fix

* https-proxy: cleanup

* server: clean up ensureUrl

* https-proxy: cleanup

* cli: fix

* cli: fix destructuring

* server: enable ForeverAgent to pool HTTPS/HTTP connections

cypress-io#3192

* server: updating snapshot

* https-proxy: don't crash, do error if proxy unreachable

* https-proxy:

* get-windows-proxy@1.0.0

* https-proxy: use proxy-from-env to decide on a proxy for a url

* server: fallback to HTTP_PROXY globally if HTTPS_PROXY not set

* server: proxy args test

* cli: add proxy tests

* cli: add test that loadSystemProxySettings is called during download

* cli, server: account for the fact that CI has some proxy vars set

* https-proxy: ""

* cli, https-proxy, server: ""

* desktop-gui: update settings gui

* desktop-gui: cypress tests for proxy settings

* server: strict undefined check

* cli, server: move get-windows-proxy to scope, optionalDeps

* server, cli: use new and improved get-windows-proxy

* cli, server: 1.5.0

* server: re-check for proxy since cli may have failed to load the lib

* server, cli: 1.5.1

* server: NO_PROXY=localhost by default, clean up

* https-proxy: disable Nagle's on proxy sockets

\cypress-io#3192

* https-proxy: use setNoDelay on upstream, cache https agent

* https-proxy: test basic auth

* https-proxy: add todo: remove this

* server: add custom HTTP(s) Agent implementation w keepalive, tunneling

* server: typescript for agent

* add ts to zunder

* server: more ts

* ts: add missing Agent type declaration

* server: create CombinedAgent

* server: use agent in more places

* ts: more declarations

* server: make script work even if debug port not supplied

* server: begin some testing

* server, ts: agent, tests

* server: test

* server: agent works with websockets now

* server: update snapshot

* server: work out some more bugs with websockets

* server: more websockets

* server: add net_profiler

* https-proxy: fix dangling socket on direct connection

* server: fix potential 'headers have already been sent'

* https-proxy: nab another dangler

* server: update test to expect agent

* https-proxy: fix failing test

* desktop-gui: change on-link

* server: add tests for empty response case

* server: tests

* server: send keep-alive with requests

* server: make net profiler hook on socket.connect

* server: only hook profiler once

* server: update tests, add keep-alive test

* server: only regen headers if needed

* server: move http_overrides into CombinedAgent, make it proxy-proof

for cypress-io#112

* server: update snapshot

* server: undo

* server: avoid circular dependency

* https-proxy, server: use our Agent instead of https-proxy-agent

* server: add dependency back

* cli: actually use proxy for download

* server, launcher, ts: typescript

* Revert "server, launcher, ts: typescript"

This reverts commit d3f8b8b.

* Revert "Revert "server, launcher, ts: typescript""

This reverts commit 818dfdf.

* ts, server: respond to PR

* server, ts: types

* ts: really fix types

* https-proxy, server: export CA from https-proxy

* agent, server, https-proxy: move agent to own package

* agent => networking, move connect into networking

* fix tests

* fix test

* networking: respond to PR changes, add more unit tests

* rename ctx

* networking, ts: add more tests

* server: add ensureUrl tests

* https-proxy: remove https-proxy-agent

* server: use CombinedAgent for API

* server: updates

* add proxy performance tests

* add perf tests to workflow

* circle

* run perf tests with --no-sandbox

* networking, ts: ch-ch-ch-ch-changes

* server, networking: pr changes

* run networking tests in circle

* server: fix performance test

* https-proxy: test that sockets are being closed

* https-proxy: write, not emit

* networking: fix test

* networking: bubble err in connect

* networking: style

* networking: clean p connect error handling

* networking => network

* server: make perf tests really work

* server: really report

* server: use args from browser

* server: use AI to determine max run time

* server: load electron only when needed


Co-authored-by: Brian Mann <brian@cypress.io>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 17, 2019

Released in 3.3.0.

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

Successfully merging a pull request may close this issue.

5 participants