-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor stencil.merger namespace #152
Conversation
ddc1786
to
73c8f5f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #152 +/- ##
=========================================
Coverage ? 93.78%
=========================================
Files ? 30
Lines ? 2077
Branches ? 92
=========================================
Hits ? 1948
Misses ? 37
Partials ? 92 ☔ View full report in Codecov by Sentry. |
2f5ea5f
to
a1a8cbb
Compare
68c7242
to
ed46cf8
Compare
Performance testing with document from issue #81 (comment
;; clj -A:build
(require '[clojure.repl.deps :refer [add-lib]])
(add-lib 'criterium/criterium {:mvn/version "0.4.6"})
(use 'criterium.core)
(require 'stencil.merger)
(->> "/home/erdos/Downloads/word/document.xml"
(clojure.java.io/file)
(clojure.java.io/reader)
(stencil.merger/parse-to-tokens-seq)
(dorun)
(bench))
) Output on current branch:
Baseline on
To summarize, there is a significant (5X) speedup. Std-deviance increased, possibly due to the GC kicking in because of the higher memory usage by the larger number of tests in a unit of time. |
Rework and simplify
merger.clj
file. This namespace is responsible for extracting stencil expressions from the template files while maintaining a correct XML tree.The file is the oldest part of the library (from 2018) and even though we barely had to touch it, it still shows signs of immaturity: complex code, bad naming, hard to follow test cases, missing docs, not very good performance.
The plan is to rewrite it to utilize transducers more with a trampoline-style executor around it. The principles of the rewrite are going to be the following: