Improve pickling - #136
Open
fingolfin wants to merge 5 commits into
Open
Conversation
IO_Pickle( ob ) and IO_Unpickle( str ) have been available since 2011 but were never documented, so nobody could be expected to find them. Document both, including that unpickling reads only the first object from the string and accepts an immutable one. The one-argument IO_Pickle also discarded the return value of the recursive call, and so returned a truncated, corrupt string instead of reporting that the object could not be pickled. It preallocated a megabyte per call as well, which for small objects costs roughly six times the runtime of starting from an empty string; the buffer grows on demand anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
IO_Unpickle stored permutations, finite field elements and cyclotomics as their printed form and read them back with EvalString, so a pickle from an untrusted source could run arbitrary code (issue #7). Functions and operations went through Read on a string, with the same effect. Write those three through formats that need no evaluation instead: PRML holds a permutation as its list of images, FFEC a field element as its coefficients over the prime field, and CYCC a cyclotomic as its coefficients. CYCC forces the question of rationals, which are cyclotomics and would otherwise pickle their own coefficients forever, so they get FRAC; infinity and -infinity get PINF and NINF. FFEC also fixes elements of large prime fields, which pickled through a printed form that GAP has no reliable method for. The old tags stay readable, but are parsed rather than evaluated: a cycle parser for PERM, and for FFEL and CYCL one recursive descent parser over integer literals, + - * / ^ and a fixed set of callable names. Field elements and cyclotomics parse in disjoint modes, which keeps the two domains from meeting and every operation total, so no malformed input can reach a break loop. Argument bounds reject a pickle whose only effect would be to exhaust memory. An operation was already stored as nothing but its name, so it needs no new format, only a lookup that does not evaluate. A function is stored as a name where it is a global and as its source otherwise; the name is resolved the same way, while the source is refused unless IO_UnpickleAllowEvalOfFunctions says otherwise. Pickles written before this change are still readable. Pickles written after it are not readable by older versions of IO, which do not know the new tags. IO_UnpickleByFunction also failed to notice a truncated pickle, and handed a short string to the function meant to interpret it; the unpicklers for integers and strings have always checked this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Only permutation and matrix groups could be pickled (issue #87). Add tags for free groups, their words, finitely presented groups, their elements, and subgroups of either. An element is only usable next to the group it came from: two elements rebuilt against two reconstructions of the same presentation land in different GAP families and cannot be multiplied. GAP records the group on the collections family of the elements family, so an element can find its own group and pickle a reference to it; the pickle cache then emits that group once per stream, and everything pickled together shares one family. Objects pickled in separate calls cannot be combined, which is inherent and is documented. Words are stored as their external representation rather than as objects, and rebuilt against a group the reader already holds. Relying on the pickle cache would not work here: IO_GenericObjectPickler writes its prepickled objects before registering its own, and at the top of a stream the cache is discarded between them, so every relator would carry, and unpickle to, a free group of its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
IO_GenericObjectPickler writes the objects an object is rebuilt from before registering the object itself, since it cannot be created without them. At the top of a stream the cache is therefore discarded between them, and two prepickled objects that are the same object in memory come back as two. This is a property of the byte format rather than of the implementation: the self-references in every pickle already written are numbered against the current behaviour, so changing when the cache is cleared would silently give them a different meaning. Say so, and say what a pickler should do instead, so that the next one written does not have to rediscover it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #75: pickle to a string.
IO_Pickle( ob )andIO_Unpickle( str )have existed since 2011 but were never documented. Now they are. The one-argument form also returned a truncated string instead of reporting failure, and preallocated 1 MB per call.Closes #7: no evaluation while unpickling.
PERM,FFELandCYCLstored printed forms read back withEvalString;FUNCandOPERusedRead. New default tagsPRML(ListPerm),FFEC(coefficients over the prime field),CYCC(CoeffsCyc), plusFRACandPINF/NINF. The old tags stay readable but are parsed: a cycle parser, and one recursive descent parser over integer literals,+ - * / ^andZ/E/ZmodpZObj/ZmodnZObj. Field elements and cyclotomics parse in disjoint modes, so every operation stays total.OPERneeded no format change.FUNCresolves globals by name; source needsIO_UnpickleAllowEvalOfFunctions := true.Old pickles still read. New pickles need IO 4.11 or newer.
Closes #87: fp groups.
FREG,FREW,FPGR,FPEL,FPSG. An element pickles a reference to its group, which GAP records on the collections family, so anything pickled together shares one family and still multiplies. Objects pickled in separate calls cannot be combined; inherent, and documented.IO_UnpickleByEvalStringis now unused; left bound and marked deprecated.Prepared with AI assistance (Claude).