Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

[WIP] Spec out AMT #137

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[WIP] Spec out AMT #137

wants to merge 3 commits into from

Conversation

dignifiedquire
Copy link
Member

Please do not review yet, still work in progress

@rvagg
Copy link
Member

rvagg commented Jul 3, 2019

doh! I have a mostly complete version of this on my machine. I was going to get it up in the next few days but I'll put up my latest at the end of my workday and we'll see how it compares. I'm calling it a Vector for now (although I do refer to AMT in the references).

@rvagg
Copy link
Member

rvagg commented Jul 3, 2019

I've put mine up at #138. If I'm reading this correctly the main difference is that I'm not bothering with the map so don't support the sparse-array case. Is there a use-case for sparse arrays? I'd be keen to avoid the complexity cost it adds unless we have a solid need for it.

@dignifiedquire
Copy link
Member Author

yeah the main use case of this structure is sparse arrays for us, the structures we expect will have sth like

1: a
2: b
5000: c
5001: d
99937: e

and we need efficient in order traversal over these, without knowing the indices upfront

@rvagg
Copy link
Member

rvagg commented Jul 4, 2019

I'm hoping we can get a SortedMap to serve this kind of case (amongst many others) sooner than later. But I guess in the meantime doing an AMT would work, it's just going to have potential for extreme unbalancing unless the sparseness is evenly distributed.

@rvagg
Copy link
Member

rvagg commented Aug 24, 2019

I've put up #180 for discussion, allowing integer keys in the HashMap spec which would give us sparse arrays but without the index-ordered traversal. I'm wondering how important the ordering is for the Filecoin use-case?

Stebalien pushed a commit to Stebalien/specs that referenced this pull request Sep 18, 2019
 rm dup protection from deal proposals
@mikeal
Copy link
Contributor

mikeal commented Oct 8, 2020

@rvagg what’s the status of this one? is this still in-line with the current Filecoin AMT?

@rvagg
Copy link
Member

rvagg commented Oct 9, 2020

Not quite. Algorithmically it's probably in line with it but the schema doesn't match.

I did a heap of doc work @ filecoin-project/go-amt-ipld#23 but can't get anyone to merge it. I was thinking of pulling some of that text into this repo to form part of a spec but it's a fair bit of work. It could probably be unified with this doc.

@warpfork
Copy link
Contributor

Hokay,,, we really gotta figure out how to close this out. It's approaching its second birthday.

I think the writeup here is very nice. However, it also seems to cover very similar information as other spec files we now have, as discussed already. If the actual data in Filecoin has evolved past this, that also reduces the utility of this document in its current form significantly.

I'm going to move to merge-ignore this (e.g. git checkout master && git merge -s ours this-branch; means: keep it in history, but don't apply the diff) if nobody else objects within a day or two.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants