Caching Strategy for SpatialRefSysMixin.srs

I am currently refactoring the srs property in the SpatialRefSysMixin class, particularly regarding the caching of the gdal.SpatialReference object. While caching can provide performance improvements, I want to ensure that we are aligning with best practices and addressing potential issues before proceeding further.

  1. Caching Necessity:
  • The gdal.SpatialReference object is computationally expensive to generate, but I’m evaluating whether caching is necessary. If the property is accessed infrequently, caching might introduce unnecessary complexity without significant performance gains.
  • We also need to consider whether the data (wkt or proj4text) changes frequently enough to require cache invalidation.
  1. Cache Invalidation:
  • If caching is retained, a mechanism for cache invalidation will be needed to ensure consistency when the underlying data changes.

I’d appreciate your feedback on whether this caching strategy is appropriate for this case, and if so, the best approach to handle invalidation.

Thank you for raising this
I believe this is your ticket: #36058 (Improve SpatialRefSysMixin.srs error reporting and efficiency) – Django

In the code, we have a 15 year old comment about whether the caching is worth the complexity:

I am tempted to be bold and suggest that it is not worth the complexity.
If we have someone raise a ticket about a performance regression, we can then re-implement caching and would know for sure that it is needed.

At the very least we could simplify the logic to use a @cached_property or something something :thinking:

Thank you @sarahboyce, for pointing out the historical context and linking the relevant ticket.

I agree that removing the caching logic altogether might simplify significantly, especially if we don’t have evidence of frequent or critical performance issues caused by the absence of caching. If performance regressions arise later, we could reintroduce caching with a more informed approach.

Using @cached_property is an excellent suggestion. It would deliver improved performance without adding the complexities of manual cache management.

I will take a closer look at how often the srs property is accessed in typical workflows to evaluate the trade-offs. Based on this discussion, I would propose removing caching entirely or replacing it with @cached_property, if there are no objections. I’ll also include a clear note in the documentation about the rationale for the change.

1 Like