Caching issue

[sbng]

I have fairly long record to encode. However, whenever I turn on cache=True, I get erroneous results after a few encoding operations. The caches seems to be pointing to the wrong data. I workaround this issue by using the self.data_cache[value] instead of self.data_cache[id(value)]. I wonder there's any shortcoming using this approach? Thanks.

[vimt]

Thank you for raising this issue. Using data_cache[value] is indeed a valid approach and can even be preferable in many cases. It's perfectly fine as long as your value is hashable.

The reason I originally used id(value) as the key was to accommodate scenarios where the cache key might be a mutable object like a dictionary or a list, which aren't hashable by default. This approach allows for more flexibility in the types of objects that can be used as keys.

However, if all your values are hashable, using data_cache[value] directly is more intuitive and can potentially avoid some pitfalls associated with using id().

Regarding the erroneous results you're experiencing, it would be extremely helpful if you could provide a specific example that demonstrates the issue. This would allow me to investigate further and potentially identify any underlying problems with the current implementation.

[sbng]

This is my input for encoding:
{ "continent": "Asia", "range": "1.0.0.0/24", "num": 100 }
{ "continent": "Africa", "range": "1.0.1.0/24", "num": 3200 }
{ "continent": "Asia", "range": "1.0.2.0/24", "num": 2300 }
{ "continent": "Europe", "range": "1.0.3.0/24", "num": 2030 }
{ "continent": "America", "range": "1.0.4.0/24", "num": 2001 }

This is my output after encoding with cache=True.
{"continent":"Asia","network":"1.0.0.0/24","num":100,"range":"1.0.0.0/24"}
{"continent":"Africa","network":"1.0.1.0/24","num":3200,"range":"1.0.1.0/24"}
{"1.0.0.0/24":"Asia","network":"1.0.2.0/24","num":100,"range":"1.0.2.0/24"}
{"continent":"Asia","network":"1.0.0.0/24","num":100,"range":"1.0.3.0/24"}
{"continent":"Africa","network":"1.0.1.0/24","num":3200,"range":"1.0.4.0/24"}

I am missing many entries. and some entries are wrong. Example, line 3. Using cache=False fixed the problem. But I have a very large data set and cache=False will have data record more then 32 bits and not feasible beyond certain entires.. So I end up doing fixing this by:

            if not isinstance(value, (list, dict)):
                return self.data_cache[value]

and corresponding code:

            if not isinstance(value, (list, dict)):
                self.data_cache[value] = pointer

I know I will not be caching (list,dict) and lower cache efficiency, but at least my data is correct. I added a "else" use id(value). The result is still not correct. Any idea?

[vimt]

Thank you for providing these detailed insights. It looks like the problem is ID reuse after destruction. This problem is well-documented and discussed in depth in this Stack Overflow question: How unique is Python's id?

After considering the problem, I've come up with a potential solution. Since list and dict are not hashable, we can convert them to tuples, which are hashable. This approach should resolve the caching issues we've been experiencing.

I've implemented this solution in a new branch. Would you mind testing it out? You can find the updated code here:

https://github.com/vimt/MaxMind-DB-Writer-python/tree/12-caching-issue

I'd greatly appreciate your feedback on this implementation. If you encounter any issues or have suggestions for improvement, please let me know.

[sbng]

Thanks for the quick fix. The patch works nicely. I did my verification and the result is similar to using cache=True. Much appreciate. Wonder if you are kind enough to commit this patch soonest?

[vimt]

I've released a new version incorporating this improvement: v0.2.4

You can upgrade to the latest version using:

pip install -U mmdb_writer

If you encounter any issues or have further suggestions, please don't hesitate to let me know. Your feedback is valuable and helps improve the library for everyone.