Compiler: no longer split blocks at fun call to propagate location #1407

hhugo · 2023-02-11T10:02:52Z

Trying to address https://discuss.ocaml.org/t/dune-build-vs-makefile/11394/9.

It should also help with #1300
and fix #747

@vouillon, the PR is not ready yet but what do you think of this approach ?

@pmwhite, this could improve your compilation time significantly.

vouillon · 2023-02-13T11:22:33Z

Maybe we should use this instead of adding a field to every constructors of type instr?

type block =
  { params : Var.t list
  ; body : (instr * loc) list
  ; branch : last * loc
  }

hhugo · 2023-02-13T12:01:52Z

Maybe we should use this instead of adding a field to every constructors of type instr?
type block =
  { params : Var.t list
  ; body : (instr * loc) list
  ; branch : last * loc
  }

Yeah, probably. Ill do the change and run a quick benchmark to make sure there performance profile is the same.

vouillon · 2023-02-13T12:09:20Z

There is a mismatch between where OCaml put events, and where the JavaScript debuggers might stop. I'm not sure what to do about that.

compiler/lib/parse_bytecode.ml

hhugo · 2023-02-13T16:48:34Z

Note that I'm still exploring the design space of this change. I'm not really happy with the mapping between debug event and instructions.

hhugo · 2023-02-13T17:38:53Z

Building the toplevel with sourcemaps goes from 37.6s to 28.2s

hhugo · 2023-02-13T17:43:34Z

There is a mismatch between where OCaml put events, and where the JavaScript debuggers might stop. I'm not sure what to do about that.

Can you give more details ? are you just referring to the fact debugger is a statement ?

hhugo · 2023-02-14T10:09:00Z

I've cleaned up the code a little and recovered few locations that initially disappeared. I'm still trying find a better way to map event to instruction.

hhugo · 2023-02-14T10:42:56Z

@pmwhite, would you be able to test this and report on both compilation time and size of generated sourcemap.
This PR both add and remove locations. I'd be curious to see if how the size of sourcemap is affected.

vouillon · 2023-02-14T17:08:24Z

There is a mismatch between where OCaml put events, and where the JavaScript debuggers might stop. I'm not sure what to do about that.

Can you give more details ? are you just referring to the fact debugger is a statement ?

What I mean is that when we single-step in a JavaScript we may stop at some locations that do not correspond to OCaml events. For instance, there is no event after an array access a.(x).
So, to build accurate sourcemaps, we need to understand where the JavaScript debuggers expects locations, and where Ocaml inserts events (and what information is provided by each event).

hhugo · 2023-02-14T17:56:52Z

The PR seems to fix two issues already

remove some completely unrelated location
fix a compilation time issue when debug is enabled

~~Maybe this PR should be only do the right think to existing well understood scenario: events after a call and event before tail calls~~

hhugo · 2023-02-15T15:41:41Z

@vouillon, can I request another round of review from you ?

Tests: Accept test for gh747

- List.filter_map is available since 4.08, no need to reimplement it - combine a usage of List.filter + List.map

The start of the closure now has a debug event

…s_of_ocaml-ppx_deriving_json, js_of_ocaml-ppx, js_of_ocaml-lwt and js_of_ocaml-compiler (5.1.0) CHANGES: ## Features/Changes * Lib: Added support for KeyboardEvent.getModifierState * Misc: bump min ocaml version to 4.08 * Misc: remove some old runtime files to support some external libs * Misc: switch to dune 3.7 * Effects: partial CPS transformation, resulting in much better performances, lower compilation time and smaller generated code * Compiler: separate compilation can now drops unused units when linking (similar to ocamlc). (ocsigen/js_of_ocaml#1378) * Compiler: specialize string to js-string conversion for all valid utf8 strings (previously just ascii) * Compiler: JavaScript files generated by `js_of_ocaml` are now UTF-8 encoded. * Compiler: use identifier for object literals when possible * Compiler: Cache function arity (the length prop of a function is slow with v8) * Compiler: The js lexer is now utf8 aware, recognize and emit utf8 ident * Compiler: Update the js lexer with new number literal syntax * Compiler: update js parser to support most es6 feature (ocsigen/js_of_ocaml#1391) * Compiler: stop parsing the builtin js runtime if not necessary * Compiler: improve js pretty printer (ocsigen/js_of_ocaml#1405) * Compiler: improve debug location and speedup compilation (ocsigen/js_of_ocaml#1407) * Toplevel: Enable separate compilation of toplevels * Runtime: js backtrace recording controled by OCAMLRUNPARAM * Runtime: support for zstd decompression of marshalled data (ocaml.5.1) (ocsigen/js_of_ocaml#12006) * Runtime: stub out custom runtime events symbols for OCaml 5.1 (ocsigen/js_of_ocaml#1414) ## Bug fixes * Effects: fix Js.export and Js.export_all to work with functions (ocsigen/js_of_ocaml#1417,ocsigen/js_of_ocaml#1377) * Sourcemap: fix incorrect sourcemap with separate compilation * Compiler: fix control flow analysis; some annotions were wrong in the runtime * Compiler: js backtrace recording respected in the js runtime and when using effects * Compiler: no longer fail on invalid source file (when the file is a directory) * Runtime: fix the compilation of some mutually recursive functions

hhugo force-pushed the fast-sm branch from fb65e5b to 0ef8728 Compare February 13, 2023 10:34

vouillon reviewed Feb 13, 2023

View reviewed changes

compiler/lib/parse_bytecode.ml Outdated Show resolved Hide resolved

vouillon reviewed Feb 13, 2023

View reviewed changes

compiler/lib/parse_bytecode.ml Outdated Show resolved Hide resolved

hhugo force-pushed the fast-sm branch from 64e005f to de9825b Compare February 13, 2023 17:04

hhugo force-pushed the fast-sm branch from 140ba99 to 645ae96 Compare February 14, 2023 08:25

hhugo requested a review from vouillon February 14, 2023 10:15

hhugo marked this pull request as ready for review February 14, 2023 10:15

hhugo force-pushed the fast-sm branch from 6b3f2a5 to bf9224e Compare February 14, 2023 10:36

hhugo force-pushed the fast-sm branch from bf9224e to 4d4a465 Compare February 14, 2023 10:44

hhugo added this to the 5.1 milestone Feb 14, 2023

vouillon closed this Feb 14, 2023

vouillon reopened this Feb 14, 2023

hhugo force-pushed the fast-sm branch 2 times, most recently from 6139e33 to c630f98 Compare February 15, 2023 01:00

hhugo force-pushed the fast-sm branch 2 times, most recently from a8fbab9 to 328c6dd Compare February 18, 2023 12:53

Compiler: change format of locs

0a4bff0

hhugo force-pushed the fast-sm branch from 328c6dd to e740219 Compare February 22, 2023 00:24

Compiler: test generated code for stdlib

89749a2

hhugo added 12 commits February 22, 2023 01:36

Compiler: new test for gh747

ee8cc09

extra test

857dd64

Compiler: no longer split blocks at fun call to propagate location

f0cbecb

Tests: Accept test for gh747

Compiler: small refactoring

40a756d

- List.filter_map is available since 4.08, no need to reimplement it - combine a usage of List.filter + List.map

Compiler: make closure start after the grab instruction

55f3df7

The start of the closure now has a debug event

Compiler: better fix the loc of the first statement of a closure

edd8b12

Compiler: accept test

e0eb9b6

Changes

21e9393

Tests: accept

79a1f88

Compiler: add timer for reading debug events

1e1228b

Compiler: cleaner output for --debug parser

469c4f0

Improve

3e13fea

hhugo force-pushed the fast-sm branch from e740219 to 3e13fea Compare February 22, 2023 00:46

hhugo merged commit cb62933 into master Feb 22, 2023

hhugo deleted the fast-sm branch February 22, 2023 00:57

hhugo mentioned this pull request Mar 7, 2023

[new release] js_of_ocaml, js_of_ocaml-tyxml, js_of_ocaml-toplevel, js_of_ocaml-ppx_deriving_json, js_of_ocaml-ppx, js_of_ocaml-lwt and js_of_ocaml-compiler (5.1.0) ocaml/opam-repository#23466

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Compiler: no longer split blocks at fun call to propagate location #1407

Compiler: no longer split blocks at fun call to propagate location #1407

Uh oh!

hhugo commented Feb 11, 2023 •

edited

Loading

Uh oh!

vouillon commented Feb 13, 2023

Uh oh!

hhugo commented Feb 13, 2023

Uh oh!

vouillon commented Feb 13, 2023

Uh oh!

Uh oh!

Uh oh!

hhugo commented Feb 13, 2023

Uh oh!

hhugo commented Feb 13, 2023

Uh oh!

hhugo commented Feb 13, 2023

Uh oh!

hhugo commented Feb 14, 2023

Uh oh!

hhugo commented Feb 14, 2023

Uh oh!

vouillon commented Feb 14, 2023

Uh oh!

hhugo commented Feb 14, 2023 •

edited

Loading

Uh oh!

hhugo commented Feb 15, 2023

Uh oh!

Uh oh!

Compiler: no longer split blocks at fun call to propagate location #1407

Compiler: no longer split blocks at fun call to propagate location #1407

Uh oh!

Conversation

hhugo commented Feb 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vouillon commented Feb 13, 2023

Uh oh!

hhugo commented Feb 13, 2023

Uh oh!

vouillon commented Feb 13, 2023

Uh oh!

Uh oh!

Uh oh!

hhugo commented Feb 13, 2023

Uh oh!

hhugo commented Feb 13, 2023

Uh oh!

hhugo commented Feb 13, 2023

Uh oh!

hhugo commented Feb 14, 2023

Uh oh!

hhugo commented Feb 14, 2023

Uh oh!

vouillon commented Feb 14, 2023

Uh oh!

hhugo commented Feb 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hhugo commented Feb 15, 2023

Uh oh!

Uh oh!

hhugo commented Feb 11, 2023 •

edited

Loading

hhugo commented Feb 14, 2023 •

edited

Loading