onflow / atree

Atree provides scalable arrays and scalable ordered maps.
https://onflow.org
Apache License 2.0
40 stars 16 forks source link

Add readonly iterators and support value mutations only from non-readonly iterators #345

Closed fxamacker closed 1 year ago

fxamacker commented 1 year ago

This PR:

Changes:

For readonly iterators, the caller is responsible for preventing changes to child containers during iteration because mutations of child containers are not guaranteed to persist.

For non-readonly iterators, two additional parameters are needed to update child container in parent map when child container is modified.


bluesign commented 1 year ago

For readonly iterators, the caller is responsible for preventing changes to child containers during iteration because mutations of child containers are not guaranteed to persist.

Is there are non-deterministic case there?

codecov-commenter commented 1 year ago

Codecov Report

Attention: 37 lines in your changes are missing coverage. Please review.

Comparison is base (8d02bbc) 62.59% compared to head (f671189) 62.62%. Report is 1 commits behind head on fxamacker/inline-array-and-map.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## fxamacker/inline-array-and-map #345 +/- ## ================================================================== + Coverage 62.59% 62.62% +0.03% ================================================================== Files 15 15 Lines 10492 10559 +67 ================================================================== + Hits 6567 6613 +46 - Misses 2989 3004 +15 - Partials 936 942 +6 ``` | [Files](https://app.codecov.io/gh/onflow/atree/pull/345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onflow) | Coverage Δ | | |---|---|---| | [array.go](https://app.codecov.io/gh/onflow/atree/pull/345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onflow#diff-YXJyYXkuZ28=) | `68.10% <80.32%> (-0.04%)` | :arrow_down: | | [map.go](https://app.codecov.io/gh/onflow/atree/pull/345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onflow#diff-bWFwLmdv) | `65.30% <70.23%> (+0.01%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/onflow/atree/pull/345/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onflow)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

fxamacker commented 1 year ago

For readonly iterators, the caller is responsible for preventing changes to child containers during iteration because mutations of child containers are not guaranteed to persist.

Is there are non-deterministic case there?

Hi @bluesign, the readonly iterators will only be used internally in Cadence to reduce overhead. So "not guaranteed to persist" won't be a problem in this case.