Skip to content

Commit

Permalink
Better serialization for ranges, domains, and iterators in non-defaul…
Browse files Browse the repository at this point in the history 
…t formats (chapel-lang#23392)

This PR improves the serialization of ranges, domains, and iterators in
non-default formats. The formatting choices here will be considered
"unstable" for the upcoming release. That said, users can at least read
and write these types in a way that is compatible with a given
serialization format, rather than having the default format appear
unexpectedly in the middle of a serialized object.

Post-2.0: determine the appropriate representation of these types
through the serialization API.

[reviewed-by @jeremiah-corrado]
  • Loading branch information
@benharsh
benharsh authored Sep 14, 2023
2 parents d9a881b + 355add6 commit ae1742f
Show file tree
Hide file tree
Showing 18 changed files with 444 additions and 26 deletions.
16 changes: 0 additions & 16 deletions modules/internal/ChapelIteratorSupport.chpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,22 +353,6 @@ module ChapelIteratorSupport {
return false;
}

proc _iteratorRecord.writeThis(f) throws {
var first: bool = true;
for e in this {
if !first then
f.write(" ");
else
first = false;
f.write(e);
}
}

@chpldoc.nodoc
proc _iteratorRecord.serialize(writer, ref serializer) throws {
writeThis(writer);
}

operator =(ref ic: _iteratorRecord, xs) {
for (e, x) in zip(ic, xs) do
e = x;
Expand Down
20 changes: 20 additions & 0 deletions modules/internal/DefaultRectangular.chpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1717,6 +1717,26 @@ module DefaultRectangular {
rwLiteral("}");
}

proc DefaultRectangularDom.dsiSerialWrite(f) throws
where _supportsSerializers(f) && f.serializerType != IO.defaultSerializer {
if chpl_warnUnstable then
compilerWarning("Serialization of rectangular domains with non-default Serializer is unstable, and may change in the future");
var ser = f.serializer.startList(f, rank);
for i in 0..<rank do ser.writeElement(dsiDim(i));
ser.endList();
}

// TODO: There is currently a bug when returning domains from
// 'deserializeFrom', so this isn't tested yet.
proc DefaultRectangularDom.dsiSerialRead(f) throws
where _supportsSerializers(f) && f.deserializerType != IO.defaultDeserializer {
if chpl_warnUnstable then
compilerWarning("Deserialization of rectangular domains with non-default Deserializer is unstable, and may change in the future");
var des = f.deserializer.startList(f);
for i in 0..<rank do ranges(i) = des.readElement(ranges(i).type);
des.endList();
}

proc DefaultRectangularDom.doiToString() {
var str = "{" + ranges(0):string;
for i in 1..<rank do
Expand Down
52 changes: 46 additions & 6 deletions modules/standard/ChapelIO.chpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,30 @@ module ChapelIO {
}
implements writeSerializable(_tuple);

proc _iteratorRecord.writeThis(f) throws {
var first: bool = true;
for e in this {
if !first then
f.write(" ");
else
first = false;
f.write(e);
}
}

@chpldoc.nodoc
proc _iteratorRecord.serialize(writer, ref serializer) throws {
if serializer.type == IO.defaultSerializer {
writeThis(writer);
} else {
if chpl_warnUnstable then
compilerWarning("Serialization of iterators with non-default Serializer is unstable, and may change in the future");
var ser = serializer.startList(writer, -1);
for e in this do ser.writeElement(e);
ser.endList();
}
}

// Moved here to avoid circular dependencies in ChapelRange
// Write implementation for ranges
// Follows operator :(range, string)
Expand Down Expand Up @@ -833,7 +857,13 @@ module ChapelIO {

@chpldoc.nodoc
proc range.serialize(writer, ref serializer) throws {
writeThis(writer);
if serializer.type == defaultSerializer {
writeThis(writer);
} else {
if chpl_warnUnstable then
compilerWarning("Serialization of ranges with non-default Serializer is unstable, and may change in the future");
writer.write(this:string);
}
}
implements writeSerializable(range);

Expand All @@ -851,7 +881,7 @@ module ChapelIO {
use strideKind;
select strides {
when one do if strideVal != 1 then expectedStride = "stride 1";
when negOne do if strideVal != 1 then expectedStride = "stride -1";
when negOne do if strideVal != -1 then expectedStride = "stride -1";
when positive do if strideVal < 0 then expectedStride = "a positive";
when negative do if strideVal > 0 then expectedStride = "a negative";
when any do;
Expand All @@ -860,8 +890,9 @@ module ChapelIO {
"for a range with strides=" + strides:string + ", expected " +
(if expectedStride.size > 2 then expectedStride + " stride"
else expectedStride) + ", got stride ", strideVal:string);

if ! hasParamStride() then
_stride = strideVal;
this = (this by strideVal):this.type;
}

if f.matchLiteral(" align ") {
Expand All @@ -870,14 +901,23 @@ module ChapelIO {
// It is valid to align any range. In this case we do not store
// the alignment at runtime because it always normalizes to 0.
} else {
_alignment = chpl__mod(alignVal, _stride);
this = (this align alignVal):this.type;
}
}
}

@chpldoc.nodoc
proc ref range.deserialize(reader, ref deserializer) throws {
readThis(reader);
if deserializer.type == IO.defaultDeserializer {
readThis(reader);
} else {
if chpl_warnUnstable then
compilerWarning("Deserialization of ranges with non-default Deserializer is unstable, and may change in the future");
const data = reader.read(string);
var f = openMemFile();
f.writer().write(data);
readThis(f.reader());
}
}
implements readDeserializable(range);

Expand All @@ -888,7 +928,7 @@ module ChapelIO {
reader: fileReader(?),
ref deserializer) throws {
this.init(idxType, bounds, strides);
this.readThis(reader);
this.deserialize(reader, deserializer);
}
implements initDeserializable(range);

Expand Down
4 changes: 2 additions & 2 deletions test/deprecated/Map/mapIterators.good
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
$CHPL_HOME/modules/internal/ChapelIteratorSupport.chpl:392: In function '_getIterator':
$CHPL_HOME/modules/internal/ChapelIteratorSupport.chpl:393: warning: 'map.these' is deprecated. Consider 'map.keys' to iterate over keys or 'map.values' to iterate over values.
$CHPL_HOME/modules/internal/ChapelIteratorSupport.chpl:376: In function '_getIterator':
$CHPL_HOME/modules/internal/ChapelIteratorSupport.chpl:377: warning: 'map.these' is deprecated. Consider 'map.keys' to iterate over keys or 'map.values' to iterate over values.
mapIterators.chpl:6: called as _getIterator(x: map(int(64),int(64),false))
mapIterators.chpl:9: warning: 'map.items' is deprecated. Consider 'map.keys' to iterate over keys or 'map.values' to iterate over values.
0
Expand Down
62 changes: 62 additions & 0 deletions test/io/serializers/basic-types.big.good
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,68 @@ red
SUCCESS
==================

===== range(int(64),both,one) =====
===== writing: =====
1..10
--------------------
05312e2e3130
====================
--- read: ---
1..10
-------------
SUCCESS
===================================

===== range(int(64),both,positive) =====
===== writing: =====
1..10 by 2
--------------------
0a312e2e31302062792032
====================
--- read: ---
1..10 by 2
-------------
SUCCESS
========================================

===== range(int(64),both,negOne) =====
===== writing: =====
1..10 by -1
--------------------
0b312e2e3130206279202d31
====================
--- read: ---
1..10 by -1
-------------
SUCCESS
======================================

===== range(int(64),both,positive) =====
===== writing: =====
1..20 by 3 align 2
--------------------
12312e2e3230206279203320616c69676e2032
====================
--- read: ---
1..20 by 3 align 2
-------------
SUCCESS
========================================

===== domain(2,int(64),one) =====
===== writing: =====
{1..10, 1..10}
--------------------
000000000000000205312e2e313005312e2e3130
====================

===== domain(2,int(64),positive) =====
===== writing: =====
{1..10 by 2, 1..10 by 2}
--------------------
00000000000000020a312e2e313020627920320a312e2e31302062792032
====================

===== SimpleRecord =====
===== writing: =====
(x = 5, y = 42.0)
Expand Down
10 changes: 10 additions & 0 deletions test/io/serializers/basic-types.chpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ proc test(val, type T = val.type) {

f.writer().withSerializer(FormatWriter).write(val);
}

// Avoid bug when reading domains for now.
if isDomainType(T) then return;

{
var readVal = f.reader().withDeserializer(FormatReader).read(T);
writeln("--- read: ---");
Expand Down Expand Up @@ -158,6 +162,12 @@ proc main() {
test((1, 2, 3));
test((1, 42.0, false));
test(colors.red);
test(1..10);
test(1..10 by 2);
test(1..10 by -1);
test(1..20 by 3 align 2);
test({1..10, 1..10});
test({1..10, 1..10} by 2);
test(new SimpleRecord(5, 42.0));
test(new CustomizedRecord(7, 3.14));
test(new GenericRecord(int, 3, 42, (1,2,3)));
Expand Down
62 changes: 62 additions & 0 deletions test/io/serializers/basic-types.default.good
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,68 @@ red
SUCCESS
==================

===== range(int(64),both,one) =====
===== writing: =====
1..10
--------------------
1..10
====================
--- read: ---
1..10
-------------
SUCCESS
===================================

===== range(int(64),both,positive) =====
===== writing: =====
1..10 by 2
--------------------
1..10 by 2
====================
--- read: ---
1..10 by 2
-------------
SUCCESS
========================================

===== range(int(64),both,negOne) =====
===== writing: =====
1..10 by -1
--------------------
1..10 by -1
====================
--- read: ---
1..10 by -1
-------------
SUCCESS
======================================

===== range(int(64),both,positive) =====
===== writing: =====
1..20 by 3 align 2
--------------------
1..20 by 3 align 2
====================
--- read: ---
1..20 by 3 align 2
-------------
SUCCESS
========================================

===== domain(2,int(64),one) =====
===== writing: =====
{1..10, 1..10}
--------------------
{1..10, 1..10}
====================

===== domain(2,int(64),positive) =====
===== writing: =====
{1..10 by 2, 1..10 by 2}
--------------------
{1..10 by 2, 1..10 by 2}
====================

===== SimpleRecord =====
===== writing: =====
(x = 5, y = 42.0)
Expand Down
62 changes: 62 additions & 0 deletions test/io/serializers/basic-types.json.good
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,68 @@ red
SUCCESS
==================

===== range(int(64),both,one) =====
===== writing: =====
1..10
--------------------
"1..10"
====================
--- read: ---
1..10
-------------
SUCCESS
===================================

===== range(int(64),both,positive) =====
===== writing: =====
1..10 by 2
--------------------
"1..10 by 2"
====================
--- read: ---
1..10 by 2
-------------
SUCCESS
========================================

===== range(int(64),both,negOne) =====
===== writing: =====
1..10 by -1
--------------------
"1..10 by -1"
====================
--- read: ---
1..10 by -1
-------------
SUCCESS
======================================

===== range(int(64),both,positive) =====
===== writing: =====
1..20 by 3 align 2
--------------------
"1..20 by 3 align 2"
====================
--- read: ---
1..20 by 3 align 2
-------------
SUCCESS
========================================

===== domain(2,int(64),one) =====
===== writing: =====
{1..10, 1..10}
--------------------
["1..10", "1..10"]
====================

===== domain(2,int(64),positive) =====
===== writing: =====
{1..10 by 2, 1..10 by 2}
--------------------
["1..10 by 2", "1..10 by 2"]
====================

===== SimpleRecord =====
===== writing: =====
(x = 5, y = 42.0)
Expand Down
Loading

0 comments on commit ae1742f

Please sign in to comment.