Generate signature map at build time#1498
Conversation
Sort order changed during refactor
WardBrian
added
cleanup
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1498 +/- ##
==========================================
- Coverage 90.46% 89.52% -0.94%
==========================================
Files 66 66
Lines 10712 9684 -1028
==========================================
- Hits 9691 8670 -1021
+ Misses 1021 1014 -7
🚀 New features to boost your workflow:
|
|
Some more timings (total wall-clock): When using OCaml 5, this advantage does shrink, I believe due to a known regression On a much larger model: OCaml 5.3 (note that on this larger model, 5.3 is faster than 4.14 on the |
nhuurre
left a comment
nhuurre
left a comment
There was a problem hiding this comment.
This is the only reason any files in test/ changed.
That's the only reason test/integration/{bad,signatures} changed. Various test/**/mir.expected files got additional parens around the newly named UnsizedType.signature.
Also test/unit/dune lost a couple of ppx_deriving, not sure why they were ever there?
Yes, this was a bit of a drive-by, but every time I learn a bit more about dune I realize we've been doing something unnecessary like over-depending on things we're not using in those modules (like in #1489). These were edited when I thought doing this would require depending on |
2 participants
This is something that came out of some profiling I did recently. Essentially, for small-medium sized models, the compiler spends most of its time just loading the
Stan_math_signaturesmodule and the large hashtable it contains of function signatures:This is particularly annoying because this hashtable is static data. After some discussions on the OCaml forums about options, the following seems like the least intrusive:
duneintegrate this into the build seamlesslyThis effectively removes 90% of the overhead of "doing nothing" in the compiler, from ~90M cycles above to ~8M cycles:
Remarkably, this also doesn't really change the binary size at all, because the generated module is only a few hundred kb as source text.
Besides making the compiler faster for end users, I also observe about a net 20% speed up of
dune runtest, which is nice as well.There are some unfortunate parts of doing this:
dunemade it necessary to move theStan_math_signaturesfile out ofMiddle-- though I had wanted to do this anyway.UnsizedType.mla few new helper definitions, which really cleaned things up, but it does mean some code in weird places got touched by this change.test/changed.Submission Checklist
Release notes
Decreased the start-up overhead of the compiler.
Copyright and Licensing
By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)