Skip to content

Commit 5d420c5

Browse files
authored
Add pickler.roundtrip() shortcut for testing pickle (#36441)
* Add pickler.roundtrip() shortcut for testing pickle `pickler.roundtrip(...)`` is equivalent to `pickler.loads(pickler.dumps(...))`, but avoids the overhead of compression. For pipelines that serialize large objects, this can significantly reduce the overhead of pipeline creation. * Fix typo * formatting
1 parent 118b3c7 commit 5d420c5

7 files changed

Lines changed: 72 additions & 37 deletions

File tree

sdks/python/apache_beam/internal/cloudpickle_pickler.py

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,28 @@ def dumps(
121121
enable_best_effort_determinism=False,
122122
config: cloudpickle.CloudPickleConfig = DEFAULT_CONFIG) -> bytes:
123123
"""For internal use only; no backwards-compatibility guarantees."""
124+
s = _dumps(o, enable_best_effort_determinism, config)
125+
126+
# Compress as compactly as possible (compresslevel=9) to decrease peak memory
127+
# usage (of multiple in-memory copies) and to avoid hitting protocol buffer
128+
# limits.
129+
# WARNING: Be cautious about compressor change since it can lead to pipeline
130+
# representation change, and can break streaming job update compatibility on
131+
# runners such as Dataflow.
132+
if use_zlib:
133+
c = zlib.compress(s, 9)
134+
else:
135+
c = bz2.compress(s, compresslevel=9)
136+
del s # Free up some possibly large and no-longer-needed memory.
137+
138+
return base64.b64encode(c)
139+
140+
141+
def _dumps(
142+
o,
143+
enable_best_effort_determinism=False,
144+
config: cloudpickle.CloudPickleConfig = DEFAULT_CONFIG) -> bytes:
145+
124146
if enable_best_effort_determinism:
125147
# TODO: Add support once https://github.com/cloudpipe/cloudpickle/pull/563
126148
# is merged in.
@@ -145,21 +167,7 @@ def dumps(
145167
if EnumDescriptor is not None:
146168
pickler.dispatch_table[EnumDescriptor] = _pickle_enum_descriptor
147169
pickler.dump(o)
148-
s = file.getvalue()
149-
150-
# Compress as compactly as possible (compresslevel=9) to decrease peak memory
151-
# usage (of multiple in-memory copies) and to avoid hitting protocol buffer
152-
# limits.
153-
# WARNING: Be cautious about compressor change since it can lead to pipeline
154-
# representation change, and can break streaming job update compatibility on
155-
# runners such as Dataflow.
156-
if use_zlib:
157-
c = zlib.compress(s, 9)
158-
else:
159-
c = bz2.compress(s, compresslevel=9)
160-
del s # Free up some possibly large and no-longer-needed memory.
161-
162-
return base64.b64encode(c)
170+
return file.getvalue()
163171

164172

165173
def loads(encoded, enable_trace=True, use_zlib=False):
@@ -173,12 +181,20 @@ def loads(encoded, enable_trace=True, use_zlib=False):
173181
s = bz2.decompress(c)
174182

175183
del c # Free up some possibly large and no-longer-needed memory.
184+
return _loads(s)
185+
176186

187+
def _loads(s):
177188
with _pickle_lock:
178189
unpickled = cloudpickle.loads(s)
179190
return unpickled
180191

181192

193+
def roundtrip(o):
194+
"""Internal utility for testing round-trip pickle serialization."""
195+
return _loads(_dumps(o))
196+
197+
182198
def _pickle_absl_flags(obj):
183199
return _create_absl_flags, tuple([])
184200

sdks/python/apache_beam/internal/dill_pickler.py

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,25 @@ def dumps(
381381
use_zlib=False,
382382
enable_best_effort_determinism=False) -> bytes:
383383
"""For internal use only; no backwards-compatibility guarantees."""
384+
s = _dumps(o, enable_trace, enable_best_effort_determinism)
385+
386+
# Compress as compactly as possible (compresslevel=9) to decrease peak memory
387+
# usage (of multiple in-memory copies) and to avoid hitting protocol buffer
388+
# limits.
389+
# WARNING: Be cautious about compressor change since it can lead to pipeline
390+
# representation change, and can break streaming job update compatibility on
391+
# runners such as Dataflow.
392+
if use_zlib:
393+
c = zlib.compress(s, 9)
394+
else:
395+
c = bz2.compress(s, compresslevel=9)
396+
del s # Free up some possibly large and no-longer-needed memory.
397+
398+
return base64.b64encode(c)
399+
400+
401+
def _dumps(o, enable_trace=True, enable_best_effort_determinism=False) -> bytes:
402+
"""For internal use only; no backwards-compatibility guarantees."""
384403
with _pickle_lock:
385404
if enable_best_effort_determinism:
386405
old_save_set = dill.dill.Pickler.dispatch[set]
@@ -400,20 +419,7 @@ def dumps(
400419
if enable_best_effort_determinism:
401420
dill.dill.pickle(set, old_save_set)
402421
dill.dill.pickle(frozenset, old_save_frozenset)
403-
404-
# Compress as compactly as possible (compresslevel=9) to decrease peak memory
405-
# usage (of multiple in-memory copies) and to avoid hitting protocol buffer
406-
# limits.
407-
# WARNING: Be cautious about compressor change since it can lead to pipeline
408-
# representation change, and can break streaming job update compatibility on
409-
# runners such as Dataflow.
410-
if use_zlib:
411-
c = zlib.compress(s, 9)
412-
else:
413-
c = bz2.compress(s, compresslevel=9)
414-
del s # Free up some possibly large and no-longer-needed memory.
415-
416-
return base64.b64encode(c)
422+
return s
417423

418424

419425
def loads(encoded, enable_trace=True, use_zlib=False):
@@ -427,7 +433,10 @@ def loads(encoded, enable_trace=True, use_zlib=False):
427433
s = bz2.decompress(c)
428434

429435
del c # Free up some possibly large and no-longer-needed memory.
436+
return _loads(s, enable_trace)
437+
430438

439+
def _loads(s, enable_trace=True):
431440
with _pickle_lock:
432441
try:
433442
return dill.loads(s)
@@ -441,6 +450,11 @@ def loads(encoded, enable_trace=True, use_zlib=False):
441450
dill.dill._trace(False) # pylint: disable=protected-access
442451

443452

453+
def roundtrip(o):
454+
"""Internal utility for testing round-trip pickle serialization."""
455+
return _loads(_dumps(o))
456+
457+
444458
def dump_session(file_path):
445459
"""For internal use only; no backwards-compatibility guarantees.
446460

sdks/python/apache_beam/internal/pickler.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ def loads(encoded, enable_trace=True, use_zlib=False):
6363
encoded, enable_trace=enable_trace, use_zlib=use_zlib)
6464

6565

66+
def roundtrip(o):
67+
"""Internal utility for testing round-trip pickle serialization."""
68+
return desired_pickle_lib.roundtrip(o)
69+
70+
6671
def dump_session(file_path):
6772
"""For internal use only; no backwards-compatibility guarantees.
6873

sdks/python/apache_beam/io/filebasedsource.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ def _get_concat_source(self) -> concat_source.ConcatSource:
147147
# with each _SingleFileSource. To prevent this FileBasedSource from having
148148
# a reference to ConcatSource (resulting in quadratic space complexity)
149149
# we clone it here.
150-
file_based_source_ref = pickler.loads(pickler.dumps(self))
150+
file_based_source_ref = pickler.roundtrip(self)
151151

152152
for file_metadata in files_metadata:
153153
file_name = file_metadata.path
@@ -284,7 +284,7 @@ def split(self, desired_bundle_size, start_offset=None, stop_offset=None):
284284
split.stop - split.start,
285285
_SingleFileSource(
286286
# Copying this so that each sub-source gets a fresh instance.
287-
pickler.loads(pickler.dumps(self._file_based_source)),
287+
pickler.roundtrip(self._file_based_source),
288288
self._file_name,
289289
split.start,
290290
split.stop,

sdks/python/apache_beam/runners/direct/transform_evaluator.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,7 @@ def start_bundle(self):
822822

823823
# TODO(aaltay): Consider storing the serialized form as an optimization.
824824
dofn = (
825-
pickler.loads(pickler.dumps(transform.dofn))
825+
pickler.roundtrip(transform.dofn)
826826
if self._perform_dofn_pickle_test else transform.dofn)
827827

828828
args = transform.args if hasattr(transform, 'args') else []

sdks/python/apache_beam/transforms/core.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3267,7 +3267,7 @@ def __init__(self):
32673267
try:
32683268
self._combine_fn_copy = copy.deepcopy(combine_fn)
32693269
except Exception:
3270-
self._combine_fn_copy = pickler.loads(pickler.dumps(combine_fn))
3270+
self._combine_fn_copy = pickler.roundtrip(combine_fn)
32713271

32723272
self.setup = self._combine_fn_copy.setup
32733273
self.create_accumulator = self._combine_fn_copy.create_accumulator
@@ -3288,7 +3288,7 @@ def __init__(self):
32883288
try:
32893289
self._combine_fn_copy = copy.deepcopy(combine_fn)
32903290
except Exception:
3291-
self._combine_fn_copy = pickler.loads(pickler.dumps(combine_fn))
3291+
self._combine_fn_copy = pickler.roundtrip(combine_fn)
32923292

32933293
self.setup = self._combine_fn_copy.setup
32943294
self.create_accumulator = self._combine_fn_copy.create_accumulator

sdks/python/apache_beam/transforms/ptransform.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -875,12 +875,12 @@ def __init__(self, fn, *args, **kwargs):
875875

876876
# Ensure fn and side inputs are picklable for remote execution.
877877
try:
878-
self.fn = pickler.loads(pickler.dumps(self.fn))
878+
self.fn = pickler.roundtrip(self.fn)
879879
except RuntimeError as e:
880880
raise RuntimeError('Unable to pickle fn %s: %s' % (self.fn, e))
881881

882-
self.args = pickler.loads(pickler.dumps(self.args))
883-
self.kwargs = pickler.loads(pickler.dumps(self.kwargs))
882+
self.args = pickler.roundtrip(self.args)
883+
self.kwargs = pickler.roundtrip(self.kwargs)
884884

885885
# For type hints, because loads(dumps(class)) != class.
886886
self.fn = self._cached_fn

0 commit comments

Comments
 (0)