Repository: dolthub/dolt
File: go/store/nbs/table_persister.go (Go)

Code before:
	for _, s := range sources {
		var uncmp uint64
		if uncmp, err = s.source.uncompressedLen(); err != nil {
			return compactionPlan{}, err
		}
		totalUncompressedData += uncmp

		index, err := s.source.index()
		if err != nil {
			return compactionPlan{}, err
		}
		// Calculate the amount of chunk data in |src|
		plan.totalCompressedData += s.dataLen
		plan.chunkCount += index.chunkCount()
	}

	lengthsPos := lengthsOffset(plan.chunkCount)
	suffixesPos := suffixesOffset(plan.chunkCount)
	sz := int(indexSize(plan.chunkCount) + footerSize)
	plan.mergedIndex, err = q.AcquireQuotaByteSlice(ctx, sz)
	if err != nil {
		return compactionPlan{}, err
	}
	acquiredBytes += sz

	prefixRecsSz := int(unsafe.Sizeof(prefixIndexRec{})) * int(plan.chunkCount)
	err = q.AcquireQuotaBytes(ctx, prefixRecsSz)
	if err != nil {
		return compactionPlan{}, err
	}
	acquiredBytes += prefixRecsSz
	prefixIndexRecs := make(prefixIndexSlice, 0, plan.chunkCount)
	var ordinalOffset uint32
	for _, sws := range plan.sources.sws {
		var index tableIndex
		index, err = sws.source.index()

		if err != nil {
			return compactionPlan{}, err
		}

		ordinals, cleanup, err := index.ordinals(ctx)
		if err != nil {
			return compactionPlan{}, err
		}
		defer cleanup()
		prefixes, cleanup, err := index.prefixes(ctx)
		if err != nil {
			return compactionPlan{}, err
		}
		defer cleanup()

Review comment (bug):
unsafe always piques some spidey senses. The only risk I can see here is in the future someone modifies the `prefixInexRec` struct to include an interface or whatever. Can you add a comment at the struct definition point to tell people to make their changes be unsafe.Sizeof friendly?

Do the same for `byteSpan`, `tableChunkRecord` and `stagedChunkRef`

Also, I found a little trick to future proof for these things changing sizes.

```go
var _ [16 - int(unsafe.Sizeof(byteSpan{}))]struct{}
```

will fail to compile if the size of byteSpan increases. Would at least be a mechanism to ensure future devs know they are touching something that shouldn't be touched. Falls apart if they remove a field tho.

Then again, I can't honestly imagine anyone would change these structs in a dangerous way. Take all of this with a grain of salt.

Code after:
		var uncmp uint64
		if uncmp, err = s.source.uncompressedLen(); err != nil {
			return compactionPlan{}, err
		}
		totalUncompressedData += uncmp

		index, err := s.source.index()
		if err != nil {
			return compactionPlan{}, err
		}
		// Calculate the amount of chunk data in |src|
		plan.totalCompressedData += s.dataLen
		plan.chunkCount += index.chunkCount()
	}

	lengthsPos := lengthsOffset(plan.chunkCount)
	suffixesPos := suffixesOffset(plan.chunkCount)
	sz := int(indexSize(plan.chunkCount) + footerSize)
	plan.mergedIndex, err = q.AcquireQuotaByteSlice(ctx, sz)
	if err != nil {
		return compactionPlan{}, err
	}
	acquiredBytes += sz

	prefixRecsSz := prefixIndexRecSize * int(plan.chunkCount)
	err = q.AcquireQuotaBytes(ctx, prefixRecsSz)
	if err != nil {
		return compactionPlan{}, err
	}
	acquiredBytes += prefixRecsSz
	prefixIndexRecs := make(prefixIndexSlice, 0, plan.chunkCount)
	var ordinalOffset uint32
	for _, sws := range plan.sources.sws {
		var index tableIndex
		index, err = sws.source.index()

		if err != nil {
			return compactionPlan{}, err
		}

		ordinals, cleanup, err := index.ordinals(ctx)
		if err != nil {
			return compactionPlan{}, err
		}
		defer cleanup()
		prefixes, cleanup, err := index.prefixes(ctx)
		if err != nil {
			return compactionPlan{}, err
		}
		defer cleanup()