On 31 Jan 2024, at 11:17, Christian Lindig <christian.lindig@xxxxxxxxx> wrote:
On 31 Jan 2024, at 10:52, Edwin Török <edwin.torok@xxxxxxxxx> wrote:
Now that we no longer have a hashtable inside we can make Quota.t pure,
and push the mutable update to its callers.
Store.t already had a mutable Quota.t field.
No functional change.
Acked-by: Christian Lindig <christian.lindig@xxxxxxxxx> This is shifting copying working to GC work, at least potentially. I would agree that this is a good trade-off and the code looks correct to me. But I think we should see more testing and benchmarking before merging this unless we are fine merging speculative improvements. — C
I’ve written this [1] microbenchmark which creates ~300_000 entries in xenstore, assigns quota to 1000 domains and then measure how long it takes to list xenstore (It doesn’t matter whether these domains exist or not). It shows a measurable improvement with this patch, once I’ve run a more realistic test on the original machine with 1000 real VMs I’ll post those results too:
On an Intel Xeon Gold 6354 @ 3.0 Ghz: * without my patch: 12.756 +- 0.152 seconds time elapsed ( +- 1.19% ) * with my patch: 5.6051 +- 0.0467 seconds time elapsed ( +- 0.83% )
[1] # cat >create.py <<EOF #!/usr/bin/env python3 from xen.lowlevel.xs import xs
xenstore = xs()
for i in range(1,1000): txn = xenstore.transaction_start() for j in range(1,10): for k in range(1,30): path=f"/quotatest/{i}/{j}/{k}/x" xenstore.write(txn, path, "val") # assign quota to domid i xenstore.set_permissions(txn, path, [{"dom": i}]) xenstore.transaction_end(txn) EOF # python3 create.py # perf stat -r 10 sh -c 'xenstore-ls $(for i in $(seq 1 100); do echo "/quotatest/$i"; done) >/dev/null’
Best regards, —Edwin | 
