Retain memory info from Alloc, fix pointer issue in merge

[ashwin95r]

This change is

[manishrjain]

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

cmd/dgraphmerge/main.go, line 137 [r1] (raw file):

      }
      copy(lastKey, top.key)

Also, lastKey = lastKey[0:len(top.key)]; otherwise, you get extra bytes hanging out towards the end of the slice.

---

cmd/dgraphmerge/main.go, line 142 [r1] (raw file):

      }
      copy(lastValue, top.value)

Same here.

---

posting/lists.go, line 184 [r1] (raw file):

  megs := ms.Alloc / (1 << 20)
  return int(megs)

Add a TODO, and mark this as a temporary hack. Specify why we're sticking to ms.Alloc for now.

---

Comments from Reviewable

[coveralls]

Coverage increased (+0.2%) to 54.378% when pulling 78aa783 on fix/rel into b62334b on master.

[ashwin95r]

Review status: 0 of 2 files reviewed at latest revision, 3 unresolved discussions.

---

cmd/dgraphmerge/main.go, line 137 [r1] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Also, lastKey = lastKey[0:len(top.key)]; otherwise, you get extra bytes hanging out towards the end of the slice.

Done.

---

cmd/dgraphmerge/main.go, line 142 [r1] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Same here.

Done.

---

posting/lists.go, line 184 [r1] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Add a TODO, and mark this as a temporary hack. Specify why we're sticking to ms.Alloc for now.

Done.

---

Comments from Reviewable

[coveralls]

Coverage decreased (-44.7%) to 9.438% when pulling 8c579d4 on fix/rel into b62334b on master.

[coveralls]

Coverage decreased (-44.7%) to 9.438% when pulling ff406c1 on fix/rel into b62334b on master.

[coveralls]

Coverage increased (+0.2%) to 54.362% when pulling ff406c1 on fix/rel into b62334b on master.

[manishrjain]

Reviewed 2 of 2 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

cmd/dgraphmerge/main.go, line 133 [r3] (raw file):

      }

      if len(lastKey) < len(top.key) {

check cap of lastKey, instead of len. https://blog.golang.org/go-slices-usage-and-internals

---

cmd/dgraphmerge/main.go, line 137 [r3] (raw file):

      }
      copy(lastKey, top.key)
      lastKey = lastKey[:len(top.key)]

Move this before copy.

---

Comments from Reviewable

[ashwin95r]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

cmd/dgraphmerge/main.go, line 133 [r3] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

check cap of lastKey, instead of len. https://blog.golang.org/go-slices-usage-and-internals

I think even if cap of lastKey is 100, but len is 10 and len of top.key is 20, only 10 elements would be copied.

https://golang.org/pkg/builtin/#copy

---

cmd/dgraphmerge/main.go, line 137 [r3] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Move this before copy.

Is there an advantage to moving it before?

---

Comments from Reviewable

[pawanrawal]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

cmd/dgraphmerge/main.go, line 133 [r3] (raw file):

Previously, ashwin95r (Ashwin Ramesh) wrote…

I think even if cap of lastKey is 100, but len is 10 and len of top.key is 20, only 10 elements would be copied.

https://golang.org/pkg/builtin/#copy

I think the point here is that if the cap of lastKey > len(top.key), you don't need to allocate lastKey again because it can store top.Key.

---

Comments from Reviewable

[ashwin95r]

Review status: 1 of 2 files reviewed at latest revision, 2 unresolved discussions.

---

cmd/dgraphmerge/main.go, line 133 [r3] (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

I think the point here is that if the cap of lastKey > len(top.key), you don't need to allocate lastKey again because it can store top.Key.

Done.

---

cmd/dgraphmerge/main.go, line 137 [r3] (raw file):

Previously, ashwin95r (Ashwin Ramesh) wrote…

Is there an advantage to moving it before?

Okay the previous and this comment together would work good. Was considering them spearately. Done.

---

Comments from Reviewable

[coveralls]

Coverage increased (+0.2%) to 54.362% when pulling bcb2b48 on fix/rel into b62334b on master.

[pawanrawal]

---

Review status: 1 of 2 files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[pawanrawal]

Reviewed 1 of 2 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[manishrjain]

---

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[ashwin95r]

Merged to master with commit 01bbadc