Re: [PR] Optimize `to_dict` Method in `GraphBinaryWriter` [tinkerpop]
Cole-Greer closed pull request #2607: Optimize `to_dict` Method in `GraphBinaryWriter` URL: https://github.com/apache/tinkerpop/pull/2607 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Optimize `to_dict` Method in `GraphBinaryWriter` [tinkerpop]
Cole-Greer commented on PR #2607: URL: https://github.com/apache/tinkerpop/pull/2607#issuecomment-2161194961 I am going to close this PR for now as it's become inactive and I'm not currently understanding the value of this caching strategy. If you believe this to be a mistake, please update with an example case which would benefit from this cache and reopen the PR. I would be happy to reconsider. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Optimize `to_dict` Method in `GraphBinaryWriter` [tinkerpop]
Cole-Greer commented on PR #2607: URL: https://github.com/apache/tinkerpop/pull/2607#issuecomment-2150516155 Hi @vatsalcode, I wanted to follow up on if you have any performance numbers for certain cases here. I'm not sure I'm understanding the which cases may benefit from this caching, as both the original data and the cache are simply python dictionaries, and over time they will tend towards having the same contents. If there are cases I'm not understanding where this caching is providing a notable performance gain, please let me know and I would be happy to consider the PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Optimize `to_dict` Method in `GraphBinaryWriter` [tinkerpop]
Cole-Greer commented on PR #2607: URL: https://github.com/apache/tinkerpop/pull/2607#issuecomment-2118414811 Hi @vatsalcode, thanks for opening this PR. I was hoping you could share a few additional details as to which scenarios you've seen significant performance gains from this caching approach. If I'm understanding this correctly, the goal is to minimize look-ups to `self.serializers`. Do you have any profiling as to which scenarios lookups to the cache are outperforming lookups to the cache? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Optimize `to_dict` Method in `GraphBinaryWriter` [tinkerpop]
vatsalcode opened a new pull request, #2607: URL: https://github.com/apache/tinkerpop/pull/2607 ### Description: This pull request optimizes the `to_dict` method in the `GraphBinaryWriter` class by introducing a cache for serializer lookups. This enhancement improves performance by reducing the need to iterate through the serializer map multiple times. ### Changes Made: 1. Added a cache (`_serializer_cache`) to store the results of serializer lookups. 2. Modified the `to_dict` method to use the cache, improving the efficiency of serializer retrieval. ### Benefits: - **Performance Improvement**: Reduces the overhead of repeatedly searching for serializers, particularly for frequently used types. - **Efficiency**: Optimizes the serialization process, making it faster and more responsive. ### Testing: - These changes are backward compatible and do not alter the existing functionality of the `GraphBinaryWriter` class. - Existing tests have been run to ensure that the class behaves as expected. ### Notes: - This improvement is part of ongoing efforts to enhance the performance and efficiency of the codebase. Thank you for considering this pull request. I look forward to your feedback! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
