I was profiling the code of org-roam-node-find and the biggest bottleneck (after the improvements to org-roam-node-list) is the processing, for each node, of the template (it is using around 80% of the total time of finding a node with my org-roam).
The main issue is that the template (eg: “${title} ${filename}”) is parsed as many times as nodes exist (using a regular expression). If this processing was done once only, it will certainly improve the performance of org-roam-node-find.
This will require to refactor the code of this function and org-roam-node–format-entry and rewrite org-roam-node-read–to-candidate
Controlled experiment on Display Format Generation with and without GC running for node size 20000
for an example display template – all other processes have been controlled for,
My conclusion is that - the functions themselves don’t take that much time - they generate a lot of garbage sure, but even if we eliminate them – the display format templates themselves will drag down the performance,
This is the impact of using two display templates , directories and title. Using only one has significant performance boosts
If we then do our calculations to make estimate for the functions in between - and estimate for their impact - it is negligible in the scale.
Excellent results - I think your experiments also corroborate my finding that any function that comes from the files.el are disgustingly slow.
Elimination thereof skyrockets performance. I think if you collect your findings more concisely you may improve the existing implementation and reduce the function times from about 0.4 sec to 0.1. This could be a significant improvement -
mainly I want to focus on this part that it should be made common knowledge among org-roam users to not use any function that comes from the files.el - so to give up about finding directory information for nodes without implementing a cache in place.
For my case - I have eliminated the entire middle circuit of generation of display template from user set options so many of these functions have been totally retired for my use case.
I think you have made significant improvement - made the middle of the circuit 4 times better nevertheless - I would if possible like a confirmation on the impact of just ripping from the code any function that comes from the said file especially here
@nobiot maybe I can bring your attention to here – removing this function from files.elfile will speed up performance – I dont understand what this line is supposed to do here – but it creates a huge latency – GC reduction as proposed by @dmg will have the most significant impact here – this is the most expensive protocol between the trio of functions that make up the read protocol –
It’s a bit surprising that this form affects performance – I think it’s used to construct each candidate displayed in org-roam-node-find based on user option org-roam-node--format-entry. So… If you did’t have ${file} as part of the user option (usually, you would not), I would have thought the form would not be evaluated and hence no performance implication…
org-roam-node-display-template is a variable defined in ‘org-roam-node.el’.
Its value is "${title} ${tags} ${file} ${todo}"
Original value was "${title}"
So my template is the one that triggers that.
To be honest, I didn’t look into the original template. If that is the case, the “default” query for the database does not need to do all those joins!
it brings an interesting point: a major rewrite would be:
the query should retrieve only what it needs (plus the id) and the completion mechanism should be used this data only (plus perhaps an ID, if one wants to dynamically show the candidates contents)
Yes, thanks for your clarification @nobiot - I have been led to believe that I need to speak more clearly, because I feel I can never get my intent across
By default it doesn’t you are correct - I discovered when I was testing a more direct route to generate the display candidate that if I used any function coming from the files.el file the performance will degrade very fast. So in the condition it gets evaluated – it shall manifest itself as a noticeable lag.
I want to ask you a final opinion - do you think a hash table here would benefit the larger community? It will have to be reset by some mechanism when display candidate format is changed – but if say we make the node struct the key and the main candidate format as the value – then we can improve read latency easily.
Between the trio of functions - the latency here will be noticed by users first. Can you think of some reasons why a hash table by default will be a bad idea – or whether it should be a user set feature – I want to ask you this because over time collective analysis has shown that trivially memoizing this part has the greatest impact for medium sized knowledge bases.
