Skip to content

Commit 74b4273

Browse files
committed
avoid creating unnecessary copies of allocating constants in field flattening
Fixes #7406 fix: avoid creating unnecessary copies of allocating constants in field flattening The field flattening optimization was creating unnecessary copies of allocating constants (like Const_block and Const_some), which could lead to memory issues. This change makes the optimization only flatten fields when the constant doesn't allocate memory.
1 parent c65c6c0 commit 74b4273

File tree

8 files changed

+78
-16
lines changed

8 files changed

+78
-16
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#### :bug: Bug fix
2929

3030
- Fix broken `bstracing` CLI location. https://github.com/rescript-lang/rescript/pull/7398
31+
- Fix field flattening optimization to avoid creating unnecessary copies of allocating constants. https://github.com/rescript-lang/rescript-compiler/pull/7421
3132

3233
#### :house: Internal
3334

compiler/core/lam_util.ml

+4-3
Original file line numberDiff line numberDiff line change
@@ -200,14 +200,15 @@ let field_flatten_get
200200
for i = 0 to Array.length fields - 1 do
201201
if fst(fields.(i)) = name then found := Ext_list.nth_opt ls i done;
202202
(match !found with
203-
| Some c -> Lam.const c
204-
| None -> lam())
203+
| Some c when not (Lam_constant.is_allocating c) -> Lam.const c
204+
| _ -> lam())
205205
| _ -> lam ()
206206
)
207207
| Some (Constant (Const_block (_,_,ls))) ->
208208
begin match Ext_list.nth_opt ls i with
209209
| None -> lam ()
210-
| Some x -> Lam.const x
210+
| Some x when not (Lam_constant.is_allocating x) -> Lam.const x
211+
| Some _ -> lam ()
211212
end
212213
| Some _
213214
| None -> lam ()

compiler/frontend/lam_constant.ml

+9
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,12 @@ let rec eq_approx (x : t) (y : t) =
100100
| _ -> false)
101101

102102
let lam_none : t = Const_js_undefined {is_unit = false}
103+
104+
let rec is_allocating (c : t) : bool =
105+
match c with
106+
| Const_some t -> is_allocating t
107+
| Const_block _ -> true
108+
| Const_js_null | Const_js_undefined _ | Const_js_true | Const_js_false
109+
| Const_int _ | Const_char _ | Const_string _ | Const_float _ | Const_bigint _
110+
| Const_pointer _ | Const_module_alias ->
111+
false

compiler/frontend/lam_constant.mli

+2
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,5 @@ type t =
5757
val eq_approx : t -> t -> bool
5858

5959
val lam_none : t
60+
61+
val is_allocating : t -> bool
+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Generated by ReScript, PLEASE EDIT WITH CARE
2+
3+
import * as Stdlib_Int from "rescript/lib/es6/Stdlib_Int.js";
4+
5+
let group = {
6+
nested: {}
7+
};
8+
9+
function fn(str) {
10+
group.nested.field = Stdlib_Int.fromString(str, undefined);
11+
}
12+
13+
let WithNestedMutableFields = {
14+
group: group,
15+
fn: fn
16+
};
17+
18+
let p = [
19+
{
20+
field: 2
21+
},
22+
""
23+
];
24+
25+
let x = p[0];
26+
27+
let y = p[0];
28+
29+
let NoOptionalFields = {
30+
p: p,
31+
x: x,
32+
y: y
33+
};
34+
35+
export {
36+
WithNestedMutableFields,
37+
NoOptionalFields,
38+
}
39+
/* No side effect */
+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
module WithNestedMutableFields = {
2+
type nested = {mutable field?: int}
3+
type group = {nested: nested}
4+
5+
let group: group = {nested: {}}
6+
7+
let fn = str => {
8+
group.nested.field = str->Int.fromString
9+
}
10+
}
11+
12+
module NoOptionalFields = {
13+
type record = {field: int}
14+
type pair = (record, string)
15+
16+
let p: pair = ({field: 2}, "")
17+
18+
let x = fst(p)
19+
let y = fst(p)
20+
}

tests/tests/src/gpr_4632.mjs

+2-9
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,7 @@ if (myList !== 0) {
1515
T0 = {
1616
myList: myList,
1717
head: 1,
18-
tail: {
19-
hd: 2,
20-
tl: /* [] */0
21-
}
18+
tail: myList.tl
2219
};
2320
} else {
2421
throw {
@@ -47,11 +44,7 @@ if (myList$1 !== 0) {
4744
if (/* [] */0 !== 0) {
4845
T1 = {
4946
myList: myList$1,
50-
h0: [
51-
1,
52-
2,
53-
3
54-
],
47+
h0: myList$1.hd,
5548
h1: /* [] */(0).hd,
5649
h2: /* [] */(0).tl
5750
};

tests/tests/src/record_debug_test.mjs

+1-4
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,7 @@ let v = {
2525

2626
let u_a = 2;
2727

28-
let u_b = {
29-
xx: 2,
30-
yy: 3
31-
};
28+
let u_b = v.b;
3229

3330
let u = {
3431
a: u_a,

0 commit comments

Comments
 (0)