Skip to content

Commit 4f56d65

Browse files
committed
Compiler: faster constant sharing
1 parent 70d1642 commit 4f56d65

File tree

6 files changed

+136
-100
lines changed

6 files changed

+136
-100
lines changed

compiler/lib/driver.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ let pack ~wrap_with_fun ~standalone { Linker.runtime_code = js; always_required_
502502
if Config.Flag.share_constant ()
503503
then (
504504
let t1 = Timer.make () in
505-
let js = (new Js_traverse.share_constant)#program js in
505+
let js = Js_traverse.share_constant js in
506506
if times () then Format.eprintf " share constant: %a@." Timer.print t1;
507507
js)
508508
else js

compiler/lib/javascript.ml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ module Num : sig
3434

3535
val to_targetint : t -> Targetint.t
3636

37+
val hash : t -> int
38+
3739
(** Predicates *)
3840

3941
val is_zero : t -> bool
@@ -42,6 +44,8 @@ module Num : sig
4244

4345
val is_neg : t -> bool
4446

47+
val equal : t -> t -> bool
48+
4549
(** Arithmetic *)
4650

4751
val add : t -> t -> t
@@ -134,6 +138,10 @@ end = struct
134138

135139
let is_neg s = Char.equal s.[0] '-'
136140

141+
let equal a b = String.equal a b
142+
143+
let hash a = String.hash a
144+
137145
let neg s =
138146
match String.drop_prefix s ~prefix:"-" with
139147
| None -> "-" ^ s

compiler/lib/javascript.mli

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ module Num : sig
3535

3636
val to_targetint : t -> Targetint.t
3737

38+
val hash : t -> int
39+
3840
(** Predicates *)
3941

4042
val is_zero : t -> bool
@@ -43,6 +45,8 @@ module Num : sig
4345

4446
val is_neg : t -> bool
4547

48+
val equal : t -> t -> bool
49+
4650
(** Arithmetic *)
4751

4852
val add : t -> t -> t

compiler/lib/js_traverse.ml

Lines changed: 114 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -769,111 +769,133 @@ class subst sub =
769769
method ident x = sub x
770770
end
771771

772-
class map_for_share_constant =
773-
object (m)
774-
inherit map as super
775-
776-
method expression e =
777-
match e with
778-
(* JavaScript engines recognize the pattern
779-
'typeof x==="number"'; if the string is shared,
780-
less efficient code is generated. *)
781-
| EBin (op, EUn (Typeof, e1), (EStr _ as e2)) ->
782-
EBin (op, EUn (Typeof, super#expression e1), e2)
783-
| EBin (op, (EStr _ as e1), EUn (Typeof, e2)) ->
784-
EBin (op, e1, EUn (Typeof, super#expression e2))
785-
(* Some js bundler get confused when the argument
786-
of 'require' is not a literal *)
787-
| ECall
788-
( EVar (S { var = None; name = Utf8 "requires"; _ })
789-
, (ANormal | ANullish)
790-
, [ Arg (EStr _) ]
791-
, _ ) -> e
792-
| _ -> super#expression e
793-
794-
(* do not replace constant in switch case *)
795-
method switch_case e =
796-
match e with
797-
| ENum _ | EStr _ -> e
798-
| _ -> m#expression e
799-
800-
method statements l =
801-
match l with
802-
| [] -> []
803-
| ((Expression_statement (EStr _), _) as prolog) :: rest ->
804-
prolog :: List.map rest ~f:(fun (x, loc) -> m#statement x, loc)
805-
| rest -> List.map rest ~f:(fun (x, loc) -> m#statement x, loc)
806-
end
807-
808-
class replace_expr f =
809-
object
810-
inherit map_for_share_constant as super
811-
812-
method expression e = try EVar (f e) with Not_found -> super#expression e
813-
end
814-
815-
let expression_equal (a : expression) b = Poly.equal a b
772+
let expression_equal (a : expression) b =
773+
match a, b with
774+
| ENum a, ENum b -> Javascript.Num.equal a b
775+
| EStr (Utf8 a), EStr (Utf8 b) -> String.equal a b
776+
| a, b -> Poly.equal a b
816777

817778
(* this optimisation should be done at the lowest common scope *)
818779

819780
module ExprTbl = Hashtbl.Make (struct
820781
type t = expression
821782

822-
let hash = Hashtbl.hash
783+
let hash = function
784+
| ENum n -> Javascript.Num.hash n
785+
| EStr (Utf8 s) -> String.hash s
786+
| e -> Hashtbl.hash e
823787

824788
let equal = expression_equal
825789
end)
826790

827-
class share_constant =
828-
object
829-
inherit map_for_share_constant as super
830-
831-
val count = ExprTbl.create 17
791+
let share_constant js =
792+
let count = ExprTbl.create 17 in
793+
let o =
794+
object (m)
795+
inherit iter as super
832796

833-
method expression e =
834-
let e =
797+
method expression e =
835798
match e with
836-
| EStr _ | ENum _ ->
837-
let n = try ExprTbl.find count e with Not_found -> 0 in
838-
ExprTbl.replace count e (n + 1);
839-
e
840-
| _ -> e
799+
(* JavaScript engines recognize the pattern
800+
'typeof x==="number"'; if the string is shared,
801+
less efficient code is generated. *)
802+
| EBin (_, EUn (Typeof, e1), EStr _) -> super#expression e1
803+
| EBin (_, EStr _, EUn (Typeof, e2)) -> super#expression e2
804+
(* Some js bundler get confused when the argument
805+
of 'require' is not a literal *)
806+
| ECall
807+
( EVar (S { var = None; name = Utf8 "requires"; _ })
808+
, (ANormal | ANullish)
809+
, [ Arg (EStr _) ]
810+
, _ ) -> ()
811+
| EStr _ | ENum _ -> (
812+
match ExprTbl.find count e with
813+
| n -> ExprTbl.replace count e (n + 1)
814+
| exception Not_found -> ExprTbl.add count e 1)
815+
| _ -> super#expression e
816+
817+
(* do not replace constant in switch case *)
818+
method switch_case e =
819+
match e with
820+
| ENum _ | EStr _ -> ()
821+
| _ -> m#expression e
822+
823+
method statements l =
824+
match l with
825+
| [] -> ()
826+
| (Expression_statement (EStr _), _) :: rest ->
827+
List.iter rest ~f:(fun (x, _) -> m#statement x)
828+
| rest -> List.iter rest ~f:(fun (x, _) -> m#statement x)
829+
end
830+
in
831+
o#program js;
832+
let all = ExprTbl.create 17 in
833+
ExprTbl.iter
834+
(fun x n ->
835+
let shareit =
836+
match x with
837+
| EStr (Utf8 s) when n > 1 ->
838+
if String.length s < 20
839+
then Some ("str_" ^ s)
840+
else Some ("str_" ^ String.sub s ~pos:0 ~len:16 ^ "_abr")
841+
| ENum s when n > 1 ->
842+
let s = Javascript.Num.to_string s in
843+
let l = String.length s in
844+
if l > 2 then Some ("num_" ^ s) else None
845+
| _ -> None
841846
in
842-
super#expression e
843-
844-
method program p =
845-
let p = super#program p in
846-
let all = ExprTbl.create 17 in
847-
ExprTbl.iter
848-
(fun x n ->
849-
let shareit =
850-
match x with
851-
| EStr (Utf8 s) when n > 1 ->
852-
if String.length s < 20
853-
then Some ("str_" ^ s)
854-
else Some ("str_" ^ String.sub s ~pos:0 ~len:16 ^ "_abr")
855-
| ENum s when n > 1 ->
856-
let s = Javascript.Num.to_string s in
857-
let l = String.length s in
858-
if l > 2 then Some ("num_" ^ s) else None
859-
| _ -> None
860-
in
861-
match shareit with
862-
| Some name ->
863-
let v = Code.Var.fresh_n name in
864-
ExprTbl.add all x (V v)
865-
| _ -> ())
866-
count;
867-
if ExprTbl.length all = 0
868-
then p
869-
else
870-
let f = ExprTbl.find all in
871-
let p = (new replace_expr f)#program p in
872-
let all =
873-
ExprTbl.fold (fun e v acc -> DeclIdent (v, Some (e, N)) :: acc) all []
874-
in
875-
(Variable_statement (Var, all), N) :: p
876-
end
847+
match shareit with
848+
| Some name ->
849+
let v = Code.Var.fresh_n name in
850+
ExprTbl.add all x (V v)
851+
| _ -> ())
852+
count;
853+
if ExprTbl.length all = 0
854+
then js
855+
else
856+
let o =
857+
object (m)
858+
inherit map as super
859+
860+
method expression e =
861+
match e with
862+
(* JavaScript engines recognize the pattern
863+
'typeof x==="number"'; if the string is shared,
864+
less efficient code is generated. *)
865+
| EBin (op, EUn (Typeof, e1), (EStr _ as e2)) ->
866+
EBin (op, EUn (Typeof, super#expression e1), e2)
867+
| EBin (op, (EStr _ as e1), EUn (Typeof, e2)) ->
868+
EBin (op, e1, EUn (Typeof, super#expression e2))
869+
(* Some js bundler get confused when the argument
870+
of 'require' is not a literal *)
871+
| ECall
872+
( EVar (S { var = None; name = Utf8 "requires"; _ })
873+
, (ANormal | ANullish)
874+
, [ Arg (EStr _) ]
875+
, _ ) -> e
876+
| (EStr _ | ENum _) as x -> (
877+
match ExprTbl.find_opt all x with
878+
| None -> super#expression x
879+
| Some v -> EVar v)
880+
| _ -> super#expression e
881+
882+
(* do not replace constant in switch case *)
883+
method switch_case e =
884+
match e with
885+
| ENum _ | EStr _ -> e
886+
| _ -> m#expression e
887+
888+
method statements l =
889+
match l with
890+
| [] -> []
891+
| ((Expression_statement (EStr _), _) as prolog) :: rest ->
892+
prolog :: List.map rest ~f:(fun (x, loc) -> m#statement x, loc)
893+
| rest -> List.map rest ~f:(fun (x, loc) -> m#statement x, loc)
894+
end
895+
in
896+
let js = o#program js in
897+
let all = ExprTbl.fold (fun e v acc -> DeclIdent (v, Some (e, N)) :: acc) all [] in
898+
(Variable_statement (Var, all), N) :: js
877899

878900
type t =
879901
{ use : IdentSet.t

compiler/lib/js_traverse.mli

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ class rename_variable : esm:bool -> object ('a)
173173
method update_state : scope -> Javascript.ident list -> Javascript.statement_list -> 'a
174174
end
175175

176-
class share_constant : mapper
176+
val share_constant : Javascript.program -> Javascript.program
177177

178178
class compact_vardecl : object ('a)
179179
inherit map

compiler/tests-compiler/jsopt.ml

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -346,10 +346,10 @@ let%expect_test "string sharing" =
346346
(function(globalThis){
347347
"use strict";
348348
var
349-
str_npi_xcf_x80 = "npi\xcf\x80",
350-
str_abcdef = "abcdef",
351349
str_abc_def = "abc\\def",
352350
str_npi = "npiπ",
351+
str_abcdef = "abcdef",
352+
str_npi_xcf_x80 = "npi\xcf\x80",
353353
runtime = globalThis.jsoo_runtime,
354354
s3 = str_abcdef,
355355
s6 = str_npi_xcf_x80,
@@ -395,7 +395,8 @@ let%expect_test "string sharing" =
395395
return;
396396
}
397397
(globalThis));
398-
//end |}];
398+
//end
399+
|}];
399400
print_program (program ~share:false ~js_string:true);
400401
[%expect
401402
{|
@@ -459,10 +460,10 @@ let%expect_test "string sharing" =
459460
(function(globalThis){
460461
"use strict";
461462
var
462-
str_npi_xcf_x80 = "npi\xcf\x80",
463-
str_abcdef = "abcdef",
464463
str_abc_def = "abc\\def",
465464
str_npi = "npiπ",
465+
str_abcdef = "abcdef",
466+
str_npi_xcf_x80 = "npi\xcf\x80",
466467
runtime = globalThis.jsoo_runtime,
467468
caml_string_of_jsbytes = runtime.caml_string_of_jsbytes,
468469
s3 = caml_string_of_jsbytes(str_abcdef),
@@ -509,7 +510,8 @@ let%expect_test "string sharing" =
509510
return;
510511
}
511512
(globalThis));
512-
//end |}];
513+
//end
514+
|}];
513515
print_program (program ~share:false ~js_string:false);
514516
[%expect
515517
{|

0 commit comments

Comments
 (0)