Presentation performance notes

[mk]

Benchmarking using the Book of Clerk @tonsky and I found that:

• image handling is ~25%, should cache it with a type holding the memory image and encoded image
• prepend-required-cljs is ~10%, can probably record the viewers at presentation time
• process-blobs is also ~10%, can also be recorded at presentation time
• bounded-count-limit is very high with 1 million, let's lower it
• ->hash-str is ~5%, we don't need a valuehash

[borkdude]

I've been looking at optimizing prepend-required-cljs by:

• threading a recording function through all the viewer stuff and calling it in present*. This was error prone. Eventually I managed to get the CLJS notebook example working though. Still failing tests. Eventually I gave up. Perhaps it's better to separate the value being presented and the options as an explicit argument? This feels less error prone to me. Branch: cljs-deps-perf.
• Next I made a second branch, cljs-deps-perf2 where I simply used a dynamic var. This was 15 minutes work to get it working, minimal code change.

But I didn't see much of a speed up compared to main:

Cljs-deps-perf2

Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 4969.766667ms.
doc->viewer
"Elapsed time: 303.502 msecs"
Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 177.288875ms.
doc->viewer
"Elapsed time: 189.958083 msecs"
Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 182.823458ms.
doc->viewer
"Elapsed time: 150.971667 msecs"
Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 174.500083ms.
doc->viewer
"Elapsed time: 134.520042 msecs"
Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 189.340666ms.
doc->viewer
"Elapsed time: 147.123417 msecs"

main:

Warning: resetting Nippy thaw for custom type with id: :java.awt.image.BufferedImage
Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 1186.872ms.
"doc->viewer"
"Elapsed time: 237.919291 msecs"
Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 188.64125ms.
"doc->viewer"
"Elapsed time: 187.301958 msecs"
Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 183.474958ms.
"doc->viewer"
"Elapsed time: 142.829042 msecs"
Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 179.999375ms.
"doc->viewer"
"Elapsed time: 165.83725 msecs"
Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 175.315625ms.
"doc->viewer"
"Elapsed time: 133.485334 msecs"
Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 166.924334ms.
"doc->viewer"
"Elapsed time: 161.703667 msecs"
Clerk evaluated '/Users/borkdude/dev/clerk/book.clj' in 162.5635ms.
"doc->viewer"
"Elapsed time: 125.714542 msecs"

With this informal benchmark, I don't see much of an interesting speedup.

This is where I decided to run clj-async-profiler myself.
It seems the above benchmarks are a bit out of date.

(ns scratch
  (:require [clj-async-profiler.core :as profiler]
            [nextjournal.clerk :as clerk] :reload-all))

(profiler/profile (dotimes [i 100] (clerk/show! "book.clj")))

(profiler/serve-ui 8080)

main flamegraph:

03-cpu-flamegraph-2025-11-04-20-34-56.html

It seems a whopping 17% is taken by weavejester dependency so maybe we should first optimize that?

prepends-cljs-deps is 3.5% in main.

Flamegraph of cljs-deps-perf2:

04-cpu-flamegraph-2025-11-04-21-01-48.html

Here it shrinks to 0.15 but is the total amount of work faster?

Rendering book.clj 20x in cljs-deps-perf2:

"Elapsed time: 8000.17 msecs"

on main:

"Elapsed time: 8200.260125 msecs"

That does seem a bit faster but maybe not significantly? Perhaps I should be using criterium here though.

[borkdude]

In summary @mk

• I propose to go with the dynamic var approach if we want to avoid postwalking unless you're really keen on avoiding it. I think it's way less error prone that threading options through values that are going to be presented. Moreover these values will also be processed by custom viewers that may not pass these options through. So maybe a dynamic var could be even the only stable option? Curious about your viewpoint. The "viewer-names" postwalk can be collapsed into the cljs-namespace recording function.

• Next optimize weavejester dependency to get that 17% down?

• Then look at the other points in the original message

[borkdude]

I also took a look at optimizing dependency/depend. This function takes up 20% of of time of a show! call. This happens because depend calculates all transitive dependencies of a dependency and then checks if the dependent is among them, to detect circularity.

I changed dependency with a third field called transitive-deps which is kept up to date every time you add a dependency.
To do this, we actually need to call transitive on dependents, which seems we're just shifting around the problem to a different place, but it is actually 5% faster when showing the book.clj notebook (so 20% of time spent in depend is reduced to 15%). I can't properly explain why this is faster though.

What is also faster is when you have the graph, calling transitive-dependenies on it is basically free, since we cached the transitive dependency graph.

All of this is in the optimize-graph-depend branch.

Before:

After:

I'll look at the other performance things soon-ish.

[borkdude]

image handling is ~25%, should cache it with a type holding the memory image and encoded image

Where do you see 25% or is this already optimized? I see around 10% for buffered-image->bytes. Is that the one?

Will try to add cache for it.

[borkdude]

Simply memoizing buffered-image->bytes takes out the 10% of time (after an initial rendering so the buffered images are in the cache).

Is that what you mean by:

should cache it with a type holding the memory image and encoded image

?

Potential downsides of using memoize: when notebooks contains lots of big images, they will remain be in memory

We could include core.cache or implemented a bounded cache ourselves.

Any thoughts on this one @mk?

Relevant commit: 57d1a33

[borkdude]

->hash-str is ~5%, we don't need a valuehash

Can you explain why you don't need a valuehash?

[borkdude]

process-blobs is only 2.5% on my machine.

[mk]

re image caching: In addition to buffered-image->bytes I guess read-image might also be worth caching. I'd want to avoid memoize because of the memory leak. Would also like to avoid a dep like core.cache. I think our cache eviction is also pretty trivial so we don't need a complex library: we only keep the results we want to cache for the currently active doc and clear them when they are no longer part of the doc.

re valuehash: I don't think we need the cryptographic safety of sha-2 as we're using the :nextjournal/hash only for the react deps in render-result. I believe MD5 is probably good enough and should be faster. Any other ideas?

[borkdude]

I guess read-image might also be worth caching

read-image doesn't show up at all in the profile after the initial render due to nippy caching I guess. given that, do you still think it's worth caching? in case people use no-cache true or so?

[borkdude]

About md5 va something else. Most modern CPUs (after 2020 let's say have dedicated SHA2 support). ChatGPT wrote a benchmark for me which outputs this on my machine.

$ clj -M /tmp/dude.clj
Hash benchmark (JVM-only)
Buffer size: 1048576 bytes
Iterations: 2000

MD5     : 0,94 GB/s
SHA-256: 3,19 GB/s
SHA-512: 1,85 GB/s
SHA-1: 3,40 GB/s

Benchmark:
dude.zip

So I think going with SHA-256 might hit the sweet-spot here? Even though it's even cryptographically strong, it's also faster than MD5 now and seems almost equally fast to SHA-1.

[borkdude]

I don't see much time spent in SHA-512 anyway, not much to be gained here?

[borkdude]

Here's the avoiding of postwalking for require-cljs and viewer-names:

#803

Turns out I was just having a typo somewhere... 🤦

If we have this in place, I can put the image caching on top of that, by storing the cache in the same atom:

https://github.com/nextjournal/clerk/compare/cache-buffered-image-bytes

Probably for process-blobs we can do a similar thing.

Then I think we ticked everything in this issue.

[borkdude]

@mk addressed all but bounded-count limit (will push that tomorrow, or you can do it, trivial change)

one thought about v/process-blobs - can we remove this call as well? I wasn't sure about that