Skip to content

Commit

Permalink
try local optimization again
Browse files Browse the repository at this point in the history
  • Loading branch information
@Simn
Simn committed Jan 18, 2024
1 parent 1c0c936 commit 0477c76
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 9 deletions.
32 changes: 23 additions & 9 deletions src/compiler/hxb/hxbWriter.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,14 +456,14 @@ type field_writer_context = {
t_pool : StringPool.t;
pos_writer : PosWriter.t;
mutable texpr_this : texpr option;
vars : (int,tvar) pool;
vars : (tvar * int) DynArray.t;
}

let create_field_writer_context pos_writer = {
t_pool = StringPool.create ();
pos_writer = pos_writer;
texpr_this = None;
vars = new pool;
vars = DynArray.create ();
}

type hxb_writer = {
Expand Down Expand Up @@ -1266,7 +1266,21 @@ module HxbWriter = struct

and write_texpr writer (fctx : field_writer_context) (e : texpr) =
let declare_var v =
Chunk.write_uleb128 writer.chunk (fctx.vars#add v.v_id v);
let index = if has_var_flag v VHxb then begin
(* Duplicate var declaration! Can happen when writing both cf_expr and cf_expr_unoptimized,
although it arguably shouldn't. In this case we don't add the var again and instead write
out the existing ID.*)
v.v_id
end else begin
let index = DynArray.length fctx.vars in
DynArray.add fctx.vars (v,v.v_id);
(* Store local index in v_id so we find it easily for all the TLocal expressions.
This is set back by the var writer in start_texpr. *)
v.v_id <- index;
add_var_flag v VHxb;
index;
end in
Chunk.write_uleb128 writer.chunk index;
Chunk.write_option writer.chunk v.v_extra (fun ve ->
Chunk.write_list writer.chunk ve.v_params (fun ttp ->
let index = writer.local_type_parameters#add ttp () in
Expand Down Expand Up @@ -1309,7 +1323,7 @@ module HxbWriter = struct
(* vars 20-29 *)
| TLocal v ->
Chunk.write_u8 writer.chunk 20;
Chunk.write_uleb128 writer.chunk (fctx.vars#get v.v_id)
Chunk.write_uleb128 writer.chunk v.v_id;
| TVar(v,None) ->
Chunk.write_u8 writer.chunk 21;
declare_var v;
Expand Down Expand Up @@ -1657,12 +1671,12 @@ module HxbWriter = struct
List.iter (fun bytes ->
Chunk.write_bytes writer.chunk (Bytes.unsafe_of_string bytes)
) items;

let items = fctx.vars#items in
Chunk.write_uleb128 writer.chunk (DynArray.length items);
DynArray.iter (fun v ->
Chunk.write_uleb128 writer.chunk (DynArray.length fctx.vars);
DynArray.iter (fun (v,v_id) ->
v.v_id <- v_id;
remove_var_flag v VHxb;
write_var writer fctx v;
) items;
) fctx.vars;
Chunk.export_data new_chunk writer.chunk;
restore(fun new_chunk -> new_chunk)
)
Expand Down
1 change: 1 addition & 0 deletions src/core/tType.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ type flag_tvar =
| VCaught
| VStatic
| VUsedByTyper (* Set if the typer looked up this variable *)
| VHxb (* Flag used by hxb *)

let flag_tvar_names = [
"VCaptured";"VFinal";"VAnalyzed";"VAssigned";"VCaught";"VStatic";"VUsedByTyper"
Expand Down

4 comments on commit 0477c76

@Simn
Copy link
Member Author

@Simn Simn commented on 0477c76 Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

  hxb                  |   0.625 |  12 |  34 |   2233 | 
    TestNumericCasts   |   0.013 |   0 |   2 |      1 | unit
    Printer            |   0.012 |   0 |   2 |      2 | haxe.macro

Well that didn't do much... but the VHxb part is actually a bugfix: Local declarations are duplicated across cf_expr and cf_expr_unoptimized, and by always writing out both we would then restore two different variables with the same v_id, which should never happen. My first attempt by using a negative ID as a flag failed because it turns out that -0 is still 0...

@kLabz
Copy link
Contributor

@kLabz kLabz commented on 0477c76 Jan 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My first attempt by using a negative ID as a flag failed because it turns out that -0 is still 0...

Yeah that was my thought when I saw it :/
How does the var flag help with this?

Diff is back to normal here 👍

@Simn
Copy link
Member Author

@Simn Simn commented on 0477c76 Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We set the flag on declared vars so we don't declare (i.e. add them to fctx.vars) again.

@Simn
Copy link
Member Author

@Simn Simn commented on 0477c76 Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current perf:

VirtualBoxVM_cT13lRBmEs

That loop_2645 must be the one in write_texpr, which aligns with both write_type_instance and write_pos showing up with higher self-times. That means I'm definitely still optimizing in the right area.

Please sign in to comment.