v0.19: Add std/markup/djot #1509
No reviewers
mighty-gerbils/contributors
Labels
No labels
UX
active development
backlog
blocker
bootstrap
bounty
bug
dependencies
discussion
documentation
duplicate
enhancement
flaky test
help wanted
invalid
javascript
question
release
tendentious
wontfix
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
mighty-gerbils/gerbil!1509
Loading…
Reference in a new issue
No description provided.
Delete branch "jay/gerbil:v0.19-djot"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adds a native Djot parser and renderer under
:std/text/markup/djot.:std/ioinput/output.Benchmarks
Complete parsing into syntax trees, source locations disabled, no rendering. Gerbil compiled with
gxc -O -exe, measured on an AMD Ryzen 9 7940HS at commitbe13dea6.Median milliseconds per parse across five timed batches. File reading and process startup are excluded; garbage collection during parsing is included.
just a readability comment: can you ask astra to make the code sparser so that it is easier to read?
@vyzo wrote in #1509 (comment):
sure thing, done. still need to do some benchmarking and write up a description. you are welcome to look over the code though, it's basically done
WIP: v0.19: Add std/markup/djotto v0.19: Add std/markup/djotactually, now working on some more optimizations, i found this https://github.com/dcampbell24/djot-implementations/
and I see we are quite a bit behind the C parser in performance. unacceptable!!
ok, let me review and i can probably give some optimization advice. the gap to C is pretty big, but let's not throw safety out the window to close it. Also it provides a good benchmark we can use for the type-gerbil optimizer work.
so is the benchmark checked in? one obvious potential pitfall is using a port for the read; you should use directly a Reader, you can get one with
call-with-file-readerfrom std/io.we probably need an optimized writer for sxml, using a BufferedWriter instead of a port. maybe a bit out of scope for this pr, but definitely relevant to benchmarking.
S-expressions are not a very efficient data representation for HTML, either, if what you're looking for is performance.
first pass review, mainly looking at how we can improve the code and its performance:
@ -0,0 +96,4 @@(footnotes : :list))transparent: #t)(defstruct (djot-paragraph djot-container)you should mark the leaves of the AST with
final: #t. results in faster predicate and accessors/mutators.also note that transparent: #t is unnecessary, it is the default.
@ -0,0 +109,4 @@(if (or (= p end) (memq (attribute-parser-state parser) '(done fail))) p(let ((c (string-ref input p))(state (attribute-parser-state parser)))(case statethis could benefit from a hash table.
@ -0,0 +154,4 @@(extra : :list := [])) => :list(render-element tag node extra (children) newlines))(condthis is a big linear scan, i think it could benefit from an interface and dispatching the prerequisite methods. probably something to measure.
@ -0,0 +620,4 @@(def (inline-container (kind : :symbol)(source :? djot-source-range)(children : :list)) => djot-container(case kindthis could probably benefit from a hash table.
@ -0,0 +7,4 @@(export string->djot);;; Public Gambit Unicode character sets have no Gerbil wrapper in v0.19.(extern namespace: #f char-set:punctuation char-set-contains?)this should be added to the runtime module in the prelude.
@ -0,0 +114,4 @@candidate)))))))(def (visit (node : djot-node)) => :void(let (identifier (node-identifier node))you have a type annotation, use dotted notation!
i agree with fare; SSXML is ancient and not the fastest of cats. We could make a properly typed xml module, to replace sxml and html could be implemented using that. probably not in this pr though, it is already large, but it could be follow up work where we can measure improvements.
now, for immediate performance improvements see my comments, and also use a Reader instead of a port directly for reading the file to benchmark, this will make an immediate (but maybe small) difference.
v0.19: Add std/markup/djotto WIP: v0.19: Add std/markup/djotWIP: v0.19: Add std/markup/djotto v0.19: Add std/markup/djot@vyzo addressed your feedback, i kept the SXML intermediary for now
there is definitely a lot of meat left on the bones in terms of performance gains, though
yeah, but at least my quick feedback already produced gains; i looked in the benchmark.md and there seems to be quite an improvement -- clearly ahead of js now.
I think maybe we could improve performance by stopping parsing strings (and prereading a giant string) and just reading bytes/utf8 characters straight from the reader. Also it is likely that location tracking is expensive, we could have a flag to turn that off. It is useful for reporting errors, but it does cost, potentially quite a bit. Can you check with astra how hard it would be to make this transition to parsing from a buffered reader? Probably not so hard. This will also substantially improve memory usage as well, those big strings are heavy and memory hungry.
next we should figure out what to do with sxml; i think std/markup/xml and std/markup/html packages with properly structured xml and html are the way to go and now we have a nice benchmark we can use to measure things. But as I said this is for another pr, you want to take it?
@vyzo yeah I can take it, and you were thinking to remove sxml yes? like fully replace with std/markup/xml?
yea, sxml is ancient and not exactly a joy to work with.
@jay what do you think of trying the parser working directly with a (buffered) reader instead of parsing strings? We need to measure its performance, and we have benchmarks already.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.