Remove unneeded base64 encoding of pickled data.

[robertwb]

---

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

• Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
• Update CHANGES.md with noteworthy changes.
• If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

[gemini-code-assist]

Summary of Changes

Hello @robertwb, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request optimizes the data serialization within the cloudpickle_pickler module by removing a redundant base64 encoding layer that was applied after compression. This change streamlines the process, allowing for direct handling of compressed data, which should lead to more efficient data handling. A potential bug has been identified where the loads function attempts to delete a non-existent variable 'c'.

Highlights

• Redundant Encoding Removed: Eliminated an unnecessary base64 encoding and decoding step from the cloudpickle_pickler module, which was previously applied after data compression.
• Serialization Optimization: Streamlined the data serialization process by directly handling compressed data, thereby reducing processing overhead and improving efficiency.
• Code Simplification: Removed the base64 import and associated encoding/decoding calls, resulting in cleaner and more concise implementations of the dumps and loads functions.
• Potential Bug Introduced: A NameError might be introduced in the loads function due to an attempt to 'del c' after the variable 'c' is no longer defined in the modified code.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[claudevdm]

Hey @robertwb , part of the effort to relax the dependency on dill is using cloudpickle to pickle deterministic types (instead of using dill)

beam/sdks/python/apache_beam/coders/coder_impl.py

Line 563 in d78d004

_pickled_types[t] = cloudpickle_pickler.dumps(

I did not disable the compression/base64 encoding in this case because a deterministic type only has to be pickled/unpickled once per process (since the pickled type is deterministic). #36032

This is to say I think removing base64 encoding will break update compatibility, because pickled types will not be base64 encoded anymore. Can we guard this behind an update compat flag? I can make this change.

Also, do you think its worth skipping the compression step in the same change for pickling these "special" types?

[robertwb]

Ah, so you're saying that the pickled bytes are used for coder comparison for update compatibility? That's potentially dangerous (pickles have low determinism guarantees) but I suppose works most of the time. In that case, yes, we'd likely need to guard this with a flag.

[claudevdm]

Historically dill was used to pickle the "special types" #14180, and values are pickled separately to reconstruct the instance

Not sure if you remember that PR, it has been a while, but it seems you were satisfied with the determinism of pickling special types like namedtuples, dataclasses etc.

Ah, so you're saying that the pickled bytes are used for coder comparison for update compatibility? That's potentially dangerous (pickles have low determinism guarantees) but I suppose works most of the time. In that case, yes, we'd likely need to guard this with a flag.

Thats right, whenever we change how the types are pickled, it breaks update compatibility for Shuffles in streaming pipelines, so we have to guard it by a flag.

[github-actions]

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

[robertwb]

OK, I'll hold off on this PR. (Looks like there's an issue with use_fake_coders not actually using fake coders that base64 was masking as well...)

[github-actions]

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.

[github-actions]

This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.