Skip to content

Commit 8881143

Browse files
authored
veb: remove 2 clones of response bodies; reduce the GC load for the most common case (fix #25423) (#26236)
1 parent 50cb9a6 commit 8881143

File tree

4 files changed

+129
-8
lines changed

4 files changed

+129
-8
lines changed

vlib/veb/context.v

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,14 @@ pub fn (mut ctx Context) send_response_to_client(mimetype string, response strin
100100
// ctx.done is only set in this function, so in order to sent a response over the connection
101101
// this value has to be set to true. Assuming the user doesn't use `ctx.conn` directly.
102102
ctx.done = true
103-
ctx.res.body = response.clone()
104103
$if veb_livereload ? {
105104
if mimetype == 'text/html' {
106105
ctx.res.body = response.replace('</html>', '<script src="/veb_livereload/${veb_livereload_server_start}/script.js"></script>\n</html>')
106+
} else {
107+
ctx.res.body = response.clone()
107108
}
109+
} $else {
110+
ctx.res.body = response.clone()
108111
}
109112
// set Content-Type and Content-Length headers
110113
mut custom_mimetype := if ctx.content_type.len == 0 { mimetype } else { ctx.content_type }
@@ -127,10 +130,8 @@ pub fn (mut ctx Context) send_response_to_client(mimetype string, response strin
127130
ctx.res.set_status(.ok)
128131
}
129132
if ctx.takeover {
130-
println('calling fast send resp')
131133
fast_send_resp(mut ctx.conn, ctx.res) or {}
132134
}
133-
ctx.res.body = ctx.res.body.clone() // !!!! TODO memory bug
134135
// result is send in `veb.v`, `handle_route`
135136
return Result{}
136137
}

vlib/veb/tests/memory_leak_test.v

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// vtest build: !docker-ubuntu-musl
2+
import os
3+
import time
4+
import net.http
5+
6+
const sport = 13085
7+
const localserver = '127.0.0.1:${sport}'
8+
const exit_after_time = 30000
9+
const vexe = os.getenv('VEXE')
10+
const vroot = os.dir(vexe)
11+
const serverexe = os.join_path(os.cache_dir(), 'veb_memory_test_server.exe')
12+
13+
fn testsuite_begin() {
14+
os.chdir(vroot) or {}
15+
if os.exists(serverexe) {
16+
os.rm(serverexe) or {}
17+
}
18+
}
19+
20+
fn testsuite_end() {
21+
if os.exists(serverexe) {
22+
os.rm(serverexe) or {}
23+
}
24+
}
25+
26+
fn test_server_compiles() {
27+
did_compile := os.system('${os.quoted_path(vexe)} -o ${os.quoted_path(serverexe)} vlib/veb/tests/memory_leak_test_server.v')
28+
assert did_compile == 0
29+
assert os.exists(serverexe)
30+
}
31+
32+
fn test_server_runs_in_background() {
33+
spawn os.system('${os.quoted_path(serverexe)} ${sport} ${exit_after_time}')
34+
time.sleep(500 * time.millisecond)
35+
}
36+
37+
fn test_large_responses_work_correctly() {
38+
// Make requests with large response bodies and verify they work
39+
request_count := 20
40+
mut successful := 0
41+
for _ in 0 .. request_count {
42+
resp := http.get('http://${localserver}/large') or { continue }
43+
if resp.status_code == 200 && resp.body.len > 50000 {
44+
successful += 1
45+
}
46+
}
47+
// Ensure most requests succeeded (allows for some network variability)
48+
assert successful >= request_count - 2, 'Only ${successful}/${request_count} requests succeeded'
49+
}
50+
51+
fn test_heap_usage_endpoint_works() {
52+
resp := http.get('http://${localserver}/heap') or {
53+
assert false, 'Failed to get heap usage: ${err}'
54+
return
55+
}
56+
heap := resp.body.i64()
57+
// Just verify we get a reasonable value (more than 1MB, less than 1GB)
58+
assert heap > 1024 * 1024, 'Heap usage ${heap} seems too low'
59+
assert heap < 1024 * 1024 * 1024, 'Heap usage ${heap} seems too high'
60+
}
61+
62+
fn test_gc_collect_endpoint_works() {
63+
resp := http.get('http://${localserver}/gc') or {
64+
assert false, 'Failed to trigger GC: ${err}'
65+
return
66+
}
67+
assert resp.status_code == 200
68+
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
module main
2+
3+
import os
4+
import veb
5+
import time
6+
7+
struct Context {
8+
veb.Context
9+
}
10+
11+
struct App {}
12+
13+
fn exit_after_timeout(timeout_in_ms int) {
14+
time.sleep(timeout_in_ms * time.millisecond)
15+
exit(0)
16+
}
17+
18+
fn main() {
19+
if os.args.len != 3 {
20+
panic('Usage: memory_leak_test_server.exe PORT TIMEOUT_IN_MILLISECONDS')
21+
}
22+
http_port := os.args[1].int()
23+
assert http_port > 0
24+
timeout := os.args[2].int()
25+
assert timeout > 0
26+
spawn exit_after_timeout(timeout)
27+
28+
mut app := &App{}
29+
veb.run_at[App, Context](mut app,
30+
host: 'localhost'
31+
port: http_port
32+
family: .ip
33+
timeout_in_seconds: 10
34+
)!
35+
}
36+
37+
pub fn (mut app App) index(mut ctx Context) veb.Result {
38+
return ctx.text('OK')
39+
}
40+
41+
@['/heap']
42+
pub fn (mut app App) heap(mut ctx Context) veb.Result {
43+
usage := gc_heap_usage()
44+
return ctx.text('${usage.heap_size}')
45+
}
46+
47+
@['/gc']
48+
pub fn (mut app App) gc_collect(mut ctx Context) veb.Result {
49+
gc_collect()
50+
return ctx.text('GC collected')
51+
}
52+
53+
@['/large']
54+
pub fn (mut app App) large(mut ctx Context) veb.Result {
55+
data := 'X'.repeat(100 * 1024)
56+
return ctx.text(data)
57+
}

vlib/veb/veb_picoev.v

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -322,11 +322,6 @@ $if !new_veb ? {
322322
pv.delete(fd)
323323
return
324324
}
325-
// TODO: At this point the Context can safely be freed when this function returns.
326-
// The user will have to clone the context if the context object should be kept.
327-
// defer {
328-
// completed_context.free()
329-
// }
330325
match completed_context.return_type {
331326
.normal {
332327
// small optimization: if the response is small write it immediately

0 commit comments

Comments
 (0)