Closed shreevatsa closed 1 year ago
Note: There was some concern about whether the changes for the wasm bindings would make things heavy, so I've put everything under a feature flag in the latest commit 3867b0e4c927209eb23779abb18d1ac18bd7a0cc. It does make things a bit uglier so it's worth thinking about whether it's worth it. For that reason, a comparison of compilation time:
✗ hyperfine --prepare "cargo clean" --cleanup "cargo clean" "cargo build --release" "cargo build --release --features=wasm_bindings"
Benchmark 1: cargo build --release
Time (mean ± σ): 13.126 s ± 0.188 s [User: 77.690 s, System: 4.831 s]
Range (min … max): 12.941 s … 13.478 s 10 runs
Benchmark 2: cargo build --release --features=wasm_bindings
Time (mean ± σ): 13.561 s ± 0.217 s [User: 85.318 s, System: 5.679 s]
Range (min … max): 13.351 s … 14.088 s 10 runs
Summary
'cargo build --release' ran
1.03 ± 0.02 times faster than 'cargo build --release --features=wasm_bindings'
And comparison of file sizes:
# ls -lS ../target/release/
# cargo build --release --features wasm_bindings >/dev/null 2>&1 && ls -lS ../target/release/
11485584 11956672 libvidyut_prakriya.rlib
2151211 2158315 test_tinantas
2150973 2157725 create_tinantas
2140445 2147517 explain_tinanta
2081789 2085821 create_krdantas
2060363 2065003 test_krdantas
2043165 2046973 explain_krdanta
1796781 1798605 explain_subanta
1437950 1445022 create_test_file
1172907 1174843 test_subantas
16741 433749 libvidyut_prakriya.dylib
3120 3182 create_test_file.d
3118 3180 create_krdantas.d
3118 3180 create_tinantas.d
3118 3180 explain_krdanta.d
3118 3180 explain_subanta.d
3118 3180 explain_tinanta.d
3114 3176 test_krdantas.d
3114 3176 test_subantas.d
3114 3176 test_tinantas.d
3050 3112 libvidyut_prakriya.d
Demo currently (temporarily) serving at https://profound-croquembouche-79b4a5.netlify.app/
For now I've decided to just merge this into the repo. PR looks good to me -- just to check, are there any other changes you want to make, or is it ready to merge?
(Another reason to merge it in as-is: using the command line to look at prakriyas is really tedious, so I'll probably use this as a debugger!)
Don't have any other changes as of now, so feel free to just merge.
On Sat, Dec 31, 2022, 19:40 Arun Prasad @.***> wrote:
(Another reason to merge it in as-is: using the command line to look at prakriyas is really tedious, so I'll probably use this as a debugger!)
— Reply to this email directly, view it on GitHub https://github.com/ambuda-org/vidyut/pull/19#issuecomment-1368341228, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF3MKWQWKFUPS7VGOEGPKDWQD4CVANCNFSM6AAAAAATNGKCBQ . You are receiving this because you authored the thread.Message ID: @.***>
GitHub did a merge commit by default, so I edited this commit to be a squash and force-pushed as 4b50c14
.
Some (already known) possible follow-up items (let me know if these should be tracked as an issue or even multiple issues):
Frontend at https://ambuda-org.github.io/vidyullekha/
Separate the ātmanepada and parasmaipada forms
Initial feedback from https://groups.google.com/g/ambuda-discuss/c/n7toiAOQ6wY and https://groups.google.com/g/sanskrit-programmers/c/si3PmK_hShQ/m/J-n1GytzCAAJ was to fetch and show the sūtra text too, and to repeat the final form at the end of the table.
When multiple forms are generated (e.g. "भवतात् , भवताद् , भवतु"), may be nice to see all their prakriya-s side by side. (Basically make the entire table cell a link/action target, rather than an individual generated form inside a table cell.)
May be nice to encode state in the URL (https://codewithhugo.com/alpinejs-x-data-watch-url/ like https://betterprogramming.pub/how-and-why-you-should-store-react-ui-state-in-the-url-f2013a204cb2)
Code:
Given that the wasm_bindings
feature does not make much of a difference to compile time or size as seen above, could probably make it not an optional feature and just compile it every time (basically revert 3867b0e4c927209eb23779abb18d1ac18bd7a0cc) — I see that's what been partially done in https://github.com/ambuda-org/vidyut/commit/e6da7860f1a079cb56b47590044c55e97828a4bf but could probably go all the way. Would have fewer (compilation) code paths / exercising that path on every build.
There's some debugging stuff I reverted in the Makefile
in https://github.com/ambuda-org/vidyut/pull/19/commits/f7abf19969bb913cade882476d284d9515837c36 related to demangling names and DWARF; see if it's of any use. (I think it can help with the Rust code showing up in Chrome Devtools, but usefulness is still limited presently.)
Yes, this all looks right to me. In terms of priority, I would rank these as:
Let's file separate tickets for these. I'll create tickets for 1 and 2, and I'll start work on 1. Can you create tickets for the others?
For wasm -- sounds good. For now, let's use the wasm macro only on simple enums. I wonder if we can get a nicer API by allowing it on everything args
-- let me know if you're curious to try it out.
For debug -- right now our goal is overall debugging, so feel free to turn on whatever makes sense.
I see (1) is filed as https://github.com/ambuda-org/vidyut/issues/20 and (2) as https://github.com/ambuda-org/vidyut/issues/21. So for the rest:
I'll look into just sending a PR for removing the separate wasm_bindings
feature, and if it doesn't turn out to be trivial, create a ticket. The debugging from Chrome Devtools we can probably look into later.
Just a demo of
derive_tinantas
for now: can do similar withderive_subantas
andderive_krdantas
later.The "Vidyullekhā" name for the demo page is just a small joke, denoting a flash of lightning, and hinting at being ullekha of Vidyut :P