fralau / mkdocs-mermaid2-plugin

A Mermaid graphs plugin for mkdocs
https://mkdocs-mermaid2.readthedocs.io
MIT License
214 stars 26 forks source link

Cache lib url check #43

Closed jannschu closed 3 years ago

jannschu commented 3 years ago

This pull request adds a simple cache to the mermaid_lib property. The reason is that for projects with many pages the url check with url_exists(...) is done redundantly. It decreased the build time for my setup significantly.

Python 3.8 has a cached_property decorator in functools but I guess that is probably not the minimal Python version you want to support. I could use functools.lru_cache if you prefer, though.

edit: Fixes #44.

fralau commented 3 years ago
  1. I find it very interesting (and a good sign) that this project is getting optimization-oriented PRs (this is the second; see: #41 ).

  2. For a question of uniformity, could I ask you to open an issue that describes the problem you are solving, and set the Linked issues in this PR?

  3. This might be a good use case for @cached_property, but the considerations of backward compatibility are valid. IMHO, using an explicit attribute is good coding practice. I tend to declare it as Null, and set it, rather than testing for its existence. But about tastes and colors... 🙂

jannschu commented 3 years ago

Ok, I added an issue and linked it.

Regarding point 3: A disadvantage of defining an attribute for the cached value in, say, __init__ with None, is that you can not distinguish the cached value from the initial value, None in this case, in the event they are equal for some reason. Another reason to only use the attribute in the property code is that in this way it is clear that it is not intended to be used in the rest of the class (matter of taste). But I hope I understood you correctly there, feel free to edit :-)

edit: forgot a "not"

fralau commented 3 years ago

@jannschu Argument taken about point 3. Which is why I concede it is a matter of taste (or rather common sense) 🙂