Skip to content
This repository was archived by the owner on Oct 4, 2020. It is now read-only.

StrMap fromFoldable stack explosion #108

Closed
matthewleon opened this issue Jun 5, 2017 · 17 comments
Closed

StrMap fromFoldable stack explosion #108

matthewleon opened this issue Jun 5, 2017 · 17 comments

Comments

@matthewleon
Copy link
Contributor

try

M.fromFoldable $ (flip Tuple) unit <<< show <$> L.range 0 9999

This works.

Now add a 9. Boom.

Data.Map maps work fine with this.

@natefaubion
Copy link
Contributor

It uses for_ which is not stack-safe. An alternative would be to fold into an Array first, and use foreachE.

@matthewleon
Copy link
Contributor Author

It uses for_ which is not stack-safe. An alternative would be to fold into an Array first, and use foreachE.

A rock and a hard place. Is there no balm in Gilead?

@matthewleon
Copy link
Contributor Author

More seriously, I feel like there has to be a better way to fix this than generating a big pile of garbage to collect. But maybe I am very wrong.

@natefaubion
Copy link
Contributor

You can also use unsafe effects with foldl/foldr.

@matthewleon
Copy link
Contributor Author

matthewleon commented Jun 5, 2017

You can also use unsafe effects with foldl/foldr.

That's like using the cheat code and saying you beat the game, though, isn't it? :)

To be fair... I guess Data.Array goes well beyond this.

@hdgarrood
Copy link
Contributor

Should this be considered a bug in the Traversable List instance?

@matthewleon
Copy link
Contributor Author

Should this be considered a bug in the Traversable List instance?

I would say so. I'm having a look at what can be done there.

@natefaubion
Copy link
Contributor

natefaubion commented Jun 5, 2017

Should this be considered a bug in the Traversable List instance?

for_ is only Foldable in any case. The stack-unsafety is in the sequencing of Applicative effects.

@natefaubion
Copy link
Contributor

That's like using the cheat code and saying you beat the game, though, isn't it? :)

You may only use foldl/foldr, but then you generate garbage by copying StrMaps. If you want to use mutation, you must sequence effects, which is not necessarily stack safe. To guarantee that, you must use MonadRec (ala purescript-safely), which has it's own overhead. Pick two: purity, stack-safety, effeciency.

@matthewleon
Copy link
Contributor Author

Pick two: purity, stack-safety, effeciency.

i-want-it-all-and-i-want-it-now

@natefaubion
Copy link
Contributor

You can also wait until proper tail-calls are implemented pervasively, and the problem will probably take care of itself.

@matthewleon
Copy link
Contributor Author

You can also wait until proper tail-calls are implemented pervasively

Is there an issue for this one? Did a search and couldn't find it.

@natefaubion
Copy link
Contributor

I was referring to tail-call support in JS engines :)

@matthewleon
Copy link
Contributor Author

I was referring to tail-call support in JS engines :)

Ah yes, indeed. How will that help with foldr though?

@michaelficarra
Copy link

Unfortunately, Safari and V8-behind-a-flag will be the only implementations for a while. Edge has inherent call stack design limitations which prevent it and Firefox has inherent security design limitations which prevent it.

matthewleon added a commit to matthewleon/purescript-maps that referenced this issue Jun 6, 2017
matthewleon added a commit to matthewleon/purescript-maps that referenced this issue Jun 6, 2017
with benchmarks that reflect no performance degradation

addresses purescript-deprecated#108
matthewleon added a commit to matthewleon/purescript-maps that referenced this issue Jun 7, 2017
with benchmarks that reflect no performance degradation

addresses purescript-deprecated#108
matthewleon added a commit to matthewleon/purescript-maps that referenced this issue Jun 7, 2017
with benchmarks that reflect no performance degradation

addresses purescript-deprecated#108
matthewleon added a commit to matthewleon/purescript-maps that referenced this issue Jun 7, 2017
paf31 pushed a commit that referenced this issue Jun 7, 2017
with benchmarks that reflect no performance degradation

addresses #108
@hdgarrood
Copy link
Contributor

Was this fixed by #110?

@natefaubion
Copy link
Contributor

Yes.

@paf31 paf31 closed this as completed Jul 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants