Switch to module-based singleton for ParslSerializer #2467
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes some problems where the class based serializer object was sometimes
serialized and sent over the network; there is no reasonable behaviour for
deserializing a singleton, but in the previous implementation, the caches used
for serialization were overwritten.
The ParslSerializer class is removed entirely and the parsl.serialize.facade
is the singleton instead. This is a more pythonic way of representing
singletons.
Use `git show -w` on this commit to see that most of the usual
diff is only whitespace indentation changes.
To identify places where a ParslSerializer object was deserialized, I added a
__setstate__ method which is called whenever a ParslSerializer is deserialized.
This fired when using monitoring, as monitoring carries round with it quite a
lot of serialization overhead.
@@ -171,3 +171,6 @@ class ParslSerializer(object):
assert len(unpacked) == 3, "Unpack expects 3 buffers, got {}".format(len(unpacked))
return unpacked
+
+ def __setstate__(self, state):
+ raise ValueError("BENC: serializer setstate called")
benclifford
added a commit
that referenced
this pull request
May 19, 2023
This inner loop populates a table of data serializers, which should happen at the same level as populating the table of code serializers, rather than as an inner loop of code serializer population. This inner loop is idempotent, so the behaviour should mostly be unchanged by this PR, but in the obscure situation that someone modifies serialisation so that it has no code serializers, only data serializers, then before this PR, I think those data serializers would not be visible to users, as the inner loop would never be executed. This was introduced by PR #2467 which moved away from using a class singleton to using the module as the singleton.
benclifford
added a commit
that referenced
this pull request
May 23, 2023
This inner loop populates a table of data serializers, which should happen at the same level as populating the table of code serializers, rather than as an inner loop of code serializer population. This inner loop is idempotent, so the behaviour should mostly be unchanged by this PR, but in the obscure situation that someone modifies serialisation so that it has no code serializers, only data serializers, then before this PR, I think those data serializers would not be visible to users, as the inner loop would never be executed. This was introduced by PR #2467 which moved away from using a class singleton to using the module as the singleton.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes some problems where the class based serializer object was sometimes serialized and sent over the network; there is no reasonable behaviour for deserializing a singleton, but in the previous implementation, the caches used for serialization were overwritten.
The ParslSerializer class is removed entirely and the parsl.serialize.facade is the singleton instead. This is a more pythonic way of representing singletons.
Use
git show -won this commit to see that most of the usual diff is only whitespace indentation changes.To identify places where a ParslSerializer object was deserialized, I added a
__setstate__method which is called whenever a ParslSerializer is deserialized.This fired when using monitoring, as monitoring carries round with it quite a lot of serialization overhead.
Type of change