mirror of
https://github.com/restic/restic.git
synced 2026-07-17 04:34:53 +00:00
Address review feedback on #21937:
- addToPacks now returns uint32 directly and the value is passed through store() to indexMap.add unchanged, instead of casting between int and uint32. - merge() appends idx2.packs to idx.packs without going through addToPacks, so it now checks the pack count against the uint32 limit itself + error if exceeded. - optimize changelog message
This commit is contained in:
@@ -0,0 +1,8 @@
|
||||
Enhancement: Reduce memory usage of the in-memory index
|
||||
|
||||
Restic keeps an index of the repository contents in memory during most
|
||||
operations. This index now requires roughly 12% less memory, which is
|
||||
especially noticeable for large repositories with many snapshots, where
|
||||
a lot of index data has to be loaded.
|
||||
|
||||
https://github.com/restic/restic/pull/21937
|
||||
@@ -1,14 +0,0 @@
|
||||
Enhancement: Reduce memory usage of the in-memory index
|
||||
|
||||
For most operations restic keeps one index entry per blob of a repository in
|
||||
memory, which is one of the largest contributors to its memory usage. Each
|
||||
entry referenced its containing pack using a machine-word-sized integer,
|
||||
even though only a small array index is needed.
|
||||
|
||||
The pack reference is now stored as a 32-bit integer. This shrinks each index
|
||||
entry from 64 to 56 bytes on 64-bit platforms, reducing the memory required
|
||||
for the index by roughly 12% (about 100 MB for a repository with ~13 million
|
||||
blobs). Repositories on disk are unaffected; only the in-memory representation
|
||||
changed.
|
||||
|
||||
https://github.com/restic/restic/pull/XXXXX
|
||||
@@ -71,18 +71,17 @@ func NewIndex() *Index {
|
||||
|
||||
// addToPacks saves the given pack ID and return the index.
|
||||
// This procedere allows to use pack IDs which can be easily garbage collected after.
|
||||
func (idx *Index) addToPacks(id restic.ID) int {
|
||||
func (idx *Index) addToPacks(id restic.ID) uint32 {
|
||||
idx.packs = append(idx.packs, id)
|
||||
packIdx := len(idx.packs) - 1
|
||||
// packIdx is stored as a uint32 in each indexEntry; guard against an
|
||||
// (unrealistic) overflow so the cast in indexMap.add can never wrap.
|
||||
if uint64(packIdx) > math.MaxUint32 {
|
||||
// packIndex is stored as a uint32 in each indexEntry; guard against
|
||||
// an (unrealistic) overflow so the cast below can never wrap.
|
||||
if uint64(len(idx.packs)) > math.MaxUint32 {
|
||||
panic("repository index pack count overflow")
|
||||
}
|
||||
return packIdx
|
||||
return uint32(len(idx.packs) - 1)
|
||||
}
|
||||
|
||||
func (idx *Index) store(packIndex int, blob pack.Blob) {
|
||||
func (idx *Index) store(packIndex uint32, blob pack.Blob) {
|
||||
// assert that offset and length fit into uint32!
|
||||
if blob.Offset > math.MaxUint32 || blob.Length > math.MaxUint32 || blob.UncompressedLength > math.MaxUint32 {
|
||||
panic("offset or length does not fit in uint32. You have packs > 4GB!")
|
||||
@@ -487,6 +486,11 @@ func (idx *Index) merge(idx2 *Index) error {
|
||||
packlen := len(idx.packs)
|
||||
// first append packs as they might be accessed when looking for duplicates below
|
||||
idx.packs = append(idx.packs, idx2.packs...)
|
||||
// packIndex is stored as a uint32 in each indexEntry; make sure the
|
||||
// merged packs list stays within that limit.
|
||||
if uint64(len(idx.packs)) > math.MaxUint32 {
|
||||
return errors.New("index merge: too many packs")
|
||||
}
|
||||
|
||||
// copy all index entries of idx2 to idx
|
||||
for typ := range idx2.byType {
|
||||
@@ -509,7 +513,7 @@ func (idx *Index) merge(idx2 *Index) error {
|
||||
for e2 := range m2.values() {
|
||||
if !hasIdenticalEntry(e2) {
|
||||
// packIndex needs to be changed as idx2.pack was appended to idx.pack, see above
|
||||
m.add(e2.id, int(e2.packIndex)+packlen, e2.offset, e2.length, e2.uncompressedLength)
|
||||
m.add(e2.id, e2.packIndex+uint32(packlen), e2.offset, e2.length, e2.uncompressedLength)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -42,7 +42,7 @@ const (
|
||||
|
||||
// add inserts an indexEntry for the given arguments into the map,
|
||||
// using id as the key.
|
||||
func (m *indexMap) add(id restic.ID, packIdx int, offset, length uint32, uncompressedLength uint32) {
|
||||
func (m *indexMap) add(id restic.ID, packIdx uint32, offset, length uint32, uncompressedLength uint32) {
|
||||
// Make sure there is enough space for the new entry.
|
||||
m.preallocate(int(m.numentries) + 1)
|
||||
|
||||
@@ -50,7 +50,7 @@ func (m *indexMap) add(id restic.ID, packIdx int, offset, length uint32, uncompr
|
||||
e, idx := m.newEntry()
|
||||
e.id = id
|
||||
e.next = m.buckets[h] // Prepend to existing chain.
|
||||
e.packIndex = uint32(packIdx)
|
||||
e.packIndex = packIdx
|
||||
e.offset = offset
|
||||
e.length = length
|
||||
e.uncompressedLength = uncompressedLength
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package index
|
||||
|
||||
import (
|
||||
"math"
|
||||
"math/rand"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -44,7 +45,7 @@ func TestIndexMapForeach(t *testing.T) {
|
||||
for i := 0; i < N; i++ {
|
||||
var id restic.ID
|
||||
id[0] = byte(i)
|
||||
m.add(id, i, uint32(i), uint32(i), uint32(i/2))
|
||||
m.add(id, uint32(i), uint32(i), uint32(i), uint32(i/2))
|
||||
}
|
||||
|
||||
seen := make(map[int]struct{})
|
||||
@@ -90,13 +91,13 @@ func TestIndexMapForeachWithID(t *testing.T) {
|
||||
|
||||
// Test insertion and retrieval of duplicates.
|
||||
for i := 0; i < ndups; i++ {
|
||||
m.add(id, i, 0, 0, 0)
|
||||
m.add(id, uint32(i), 0, 0, 0)
|
||||
}
|
||||
|
||||
for i := 0; i < 100; i++ {
|
||||
var otherid restic.ID
|
||||
r.Read(otherid[:])
|
||||
m.add(otherid, -1, 0, 0, 0)
|
||||
m.add(otherid, math.MaxUint32, 0, 0, 0)
|
||||
}
|
||||
|
||||
n = 0
|
||||
|
||||
Reference in New Issue
Block a user