Skip to content

Commit

Permalink
Remove caching of Seqs
Browse files Browse the repository at this point in the history
This is a temporary partial commit. If you see it on master then
things went very wrong.
  • Loading branch information
AlexDaniel committed Jul 7, 2020
1 parent a745505 commit c9a4f07
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 6 deletions.
30 changes: 30 additions & 0 deletions src/core.c/Seq.pm6
Expand Up @@ -5,6 +5,9 @@ my class Seq is Cool does Iterable does Sequence {
# way through. Can only be obtained once.
has Iterator $!iter;

# The number of values that have been produced
has int $!produced;

# The only valid way to create a Seq directly is by giving it the
# iterator it will consume and maybe memoize.
multi method new(Seq: Iterator:D $iter) {
Expand Down Expand Up @@ -41,6 +44,33 @@ my class Seq is Cool does Iterable does Sequence {
)
}

proto method AT-POS(|) {*}
multi method AT-POS(Seq:D: Int:D $idx) is raw {
if $idx < $!produced {

This comment has been minimized.

Copy link
@jnthn

jnthn Jul 7, 2020

Member

I think this will mean that some-seq[@foo] will only work if @foo is ordered? Even accepting that restriction, some-seq[* - 2], which wants the elems (and so much produce everything), is going to be problematic with this too. I dunno if a more fruitful direction would be a Sequence overload on the postcircumfix candidates...

This comment has been minimized.

Copy link
@AlexDaniel

AlexDaniel Jul 7, 2020

Author Contributor

@jnthn Correct. I'm just testing if this is going to work at all. It seems like it does. Next step is to see how many modules are wrecked with this simple change (luckily Blin can now work with branches). If the whole ecosystem expects Seqs to be cacheable then this proposed change is in trouble. If not, then we can perhaps think about the right way to do it.

die "asked $idx, which was already produced";
}
else {
my int $skip = $idx - $!produced;
$!produced = $idx;
$!iter.skip($skip)
?? $!iter.pull-one
!! Nil
}
}

multi method AT-POS(Seq:D: int $idx) is raw {
if $idx < $!produced {
die "asked $idx, which was already produced";
}
else {
my int $skip = $idx - $!produced;
$!produced = $idx;
$!iter.skip($skip)
?? $!iter.pull-one
!! Nil
}
}

multi method Seq(Seq:D:) { self }

method Capture() {
Expand Down
12 changes: 6 additions & 6 deletions src/core.c/Sequence.pm6
Expand Up @@ -57,13 +57,13 @@ my role Sequence does PositionalBindFailover {

method Numeric(::?CLASS:D:) { self.cache.elems }

multi method AT-POS(::?CLASS:D: Int:D $idx) is raw {
self.cache.AT-POS($idx)
}
#multi method AT-POS(::?CLASS:D: Int:D $idx) is raw {

This comment has been minimized.

Copy link
@lizmat

lizmat Jul 7, 2020

Contributor

You are changing the semantics of the Sequence role here, is that what you intended?

This comment has been minimized.

Copy link
@AlexDaniel

AlexDaniel Jul 7, 2020

Author Contributor

This commit doesn't really make things work, I'm just playing around with things for now. But, also, in the final form the Sequence role won't have cache semantics. So yes, intended but incomplete.

# self.cache.AT-POS($idx)
#}

multi method AT-POS(::?CLASS:D: int $idx) is raw {
self.cache.AT-POS($idx)
}
#multi method AT-POS(::?CLASS:D: int $idx) is raw {
# self.cache.AT-POS($idx)
#}

multi method EXISTS-POS(::?CLASS:D: Int:D $idx) {
self.cache.EXISTS-POS($idx)
Expand Down

3 comments on commit c9a4f07

@lizmat
Copy link
Contributor

@lizmat lizmat commented on c9a4f07 Jul 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.raku.org/type/Sequence documents it providing AT-POS

@lizmat
Copy link
Contributor

@lizmat lizmat commented on c9a4f07 Jul 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And don't forget roast :-)

@AlexDaniel
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roast is a later step. Of course the spec expects Seq to behave a certain way, I'm proposing to change the spec.

Please sign in to comment.