feat: simulate real-world transport (TCP/TLS overhead, connection setup, slow-start curve) + raw UDP test - #845
Closed
Gutouoff wants to merge 2 commits into
Closed
Conversation
…FB, slow-start curve) - Estimate wire line rate from an Ethernet+IP+TCP+TLS+ACK overhead model instead of the fixed 1.06 factor; report payload throughput, line rate and overhead % separately (dlPayloadStatus/ulPayloadStatus, dlOverheadPct/ulOverheadPct). - Report TCP/TLS handshake, TTFB and negotiated protocol via Performance Resource Timing (collectNetworkInfo). - Add download/upload throughput-over-time curves showing the slow-start ramp (dlCurve/ulCurve), rendered in classic and modern UI. - Backend: add Timing-Allow-Origin header to garbage.php/empty.php/getIP.php for cross-origin timing. - Document new settings and metrics in doc.md and settings.json.
Contributor
PR Summary by QodoModel real-world transport metrics and slow-start curves
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Contributor
Code Review by Qodo
1. Curve speed assumes exact interval
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+501
to
+504
| const curveDelta = totLoaded - curveLast; | ||
| curveLast = totLoaded; | ||
| if (curveDelta >= 0) { | ||
| dlCurve.push({ t: new Date().getTime() - curveStartT, speed: (curveDelta / 0.2) * 8 / (settings.useMebibits ? 1048576 : 1000000) }); |
Contributor
There was a problem hiding this comment.
1. Curve speed assumes exact interval 🐞 Bug ≡ Correctness
The curve calculation always divides transferred bytes by 0.2 seconds even though the interval callback can run later than 200 ms due to worker load or timer throttling. Delayed callbacks therefore overstate instantaneous throughput (for example, a 400 ms interval is reported at roughly twice its actual speed), making the new slow-start curve inaccurate.
Agent Prompt
## Issue description
Curve throughput is calculated with a fixed 0.2-second denominator, although timer callbacks do not execute at exact 200 ms intervals.
## Issue Context
Track the timestamp of the previous sample separately for download and upload, and divide each byte delta by the actual elapsed duration. A monotonic clock such as `performance.now()` is preferable.
## Fix Focus Areas
- speedtest_worker.js[435-436]
- speedtest_worker.js[500-504]
- speedtest_worker.js[574-575]
- speedtest_worker.js[671-675]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+409
to
+412
| if (testState.testData.dlPayloadStatus) { | ||
| dlPayload.textContent = testState.testData.dlPayloadStatus; | ||
| dlLine.textContent = testState.testData.dlStatus; | ||
| dlOverhead.textContent = testState.testData.dlOverheadPct || "0"; |
Contributor
There was a problem hiding this comment.
2. Upload-only details stay hidden 🐞 Bug ≡ Correctness
The modern renderer reveals and populates the transport panel only when dlPayloadStatus is truthy, even when valid upload payload data and an upload curve exist. Configurations that run only U, which test_order explicitly supports, therefore never display any of the new upload transport results or chart.
Agent Prompt
## Issue description
The transport panel is gated exclusively on download payload data, hiding valid upload-only results.
## Issue Context
`test_order` supports enabling only selected tests. Reveal the panel when either direction has data, and populate/hide each direction independently so absent download values do not block upload metrics or the chart.
## Fix Focus Areas
- frontend/javascript/index.js[408-417]
- frontend/javascript/index.js[519-530]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+418
to
+425
| if ( | ||
| testState.testData.tcpHandshakeMs || | ||
| testState.testData.tlsHandshakeMs || | ||
| testState.testData.ttfbMs | ||
| ) { | ||
| connTcp.textContent = testState.testData.tcpHandshakeMs || "0"; | ||
| connTls.textContent = testState.testData.tlsHandshakeMs || "0"; | ||
| connTtfb.textContent = testState.testData.ttfbMs || "0"; |
Contributor
There was a problem hiding this comment.
3. Modern details retain old results 🐞 Bug ≡ Correctness
The modern renderer only writes connection fields when new values are nonzero and never clears or hides the transport panel when a restarted or aborted test reports empty optional metrics. Results from the previous run can therefore remain displayed indefinitely, especially when the next run has unavailable connection timing or omits download/upload tests.
Agent Prompt
## Issue description
Transport and connection DOM state survives across test runs when the next status contains empty optional values.
## Issue Context
Each `Speedtest.start()` creates a new worker whose optional metrics begin empty, and abort also clears them. Reset the panel, values, protocol, and chart when starting a new run and explicitly clear/hide fields when current status data is absent.
## Fix Focus Areas
- frontend/javascript/index.js[79-85]
- frontend/javascript/index.js[408-430]
- frontend/javascript/index.js[514-529]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+230
to
+233
| if (txt) { | ||
| I("detailsArea").style.display = ""; | ||
| I("detailsText").innerHTML = txt; | ||
| } |
Contributor
There was a problem hiding this comment.
4. Classic details retain old results 🐞 Bug ≡ Correctness
The classic renderer updates detailsText only when the newly built string is nonempty and never hides or clears the details area otherwise. After restart or abort, old transport results remain visible until a later enabled throughput test produces data, and persist for the entire run when the next configuration is ping/IP-only.
Agent Prompt
## Issue description
The classic transport panel preserves content from a previous run when current status has no transport details.
## Issue Context
Add an explicit empty-data branch that clears `detailsText`, hides `detailsArea`, and hides/clears the curve canvas. Also reset this state when starting or aborting a test.
## Fix Focus Areas
- index-classic.html[163-201]
- index-classic.html[217-234]
- index-classic.html[236-243]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Self-contained zero-dependency Node.js UDP test (mini-iperf3 style) under udp/: - udp-server.js: raw UDP server (echo ping, count upload, flood download) - udp-client.js: CLI client measuring ping/jitter/loss plus upload/download throughput - protocol.js: shared 13-byte datagram header (type+seq+timestamp) - README.md and doc.md: usage, options, and mapping to LibreSpeed metrics Browsers cannot open raw UDP sockets, so this runs as a Node.js pair rather than in speedtest_worker.js.
Gutouoff
changed the title
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Simulates real-world public-network transfer on top of LibreSpeed's existing XHR-based test, without changing the protocol stack (pure browser + an optional
Timing-Allow-Originheader). Also adds a raw-UDP test for the UDP path that browsers cannot cover.What changed
Transport overhead & connection setup (browser)
overhead_auto). The worker separately reports:dlPayloadStatus/ulPayloadStatus)dlStatus/ulStatus)dlOverheadPct/ulOverheadPct)tcpHandshakeMs,tlsHandshakeMs,ttfbMs,nextHopProtocol).dlCurve/ulCurve), rendered as a chart in both the classic and modern UI.Timing-Allow-Origin: *ongarbage.php/empty.php/getIP.phpso cross-origin (multiple points of test) timings are exposed.doc.md; new defaults insettings.json.Raw UDP test (
udp/)udp/udp-server.js— raw UDP server (echo ping, count upload, flood download)udp/udp-client.js— CLI client measuring ping/jitter/loss plus upload/download throughputudp/protocol.js— shared 13-byte datagram header (type + seq + timestamp)udp/README.md— usage and optionsspeedtest_worker.js. A browser-native UDP path would need WebTransport over HTTP/3 (QUIC) and is intentionally out of scope here.Notes