gnovm/pkg/gnolang: ReadMemPackage should reuse byteslices from reading files as those byteslices are not modified; without incurring expensive byteslice->string conversion #3598

odeke-em · 2025-01-23T18:52:10Z

While studying ReadMemPackage* I examined some profiles and noticed that we've got this RAM hog

ROUTINE ======================== github.com/gnolang/gno/gnovm/pkg/gnolang.ReadMemPackageFromList in /Users/emmanuelodeke/go/src/github.com/gnolang/gno/gnovm/pkg/gnolang/nodes.go
    3.44GB     7.20GB (flat, cum) 96.70% of Total
         .          .   1309:func ReadMemPackageFromList(list []string, pkgPath string) (*gnovm.MemPackage, error) {
       6MB        6MB   1310:	memPkg := &gnovm.MemPackage{Path: pkgPath}
         .          .   1311:	var pkgName Name
         .          .   1312:	for _, fpath := range list {
         .          .   1313:		fname := filepath.Base(fpath)
         .     2.32GB   1314:		bz, err := os.ReadFile(fpath)
         .          .   1315:		if err != nil {
         .          .   1316:			return nil, err
         .          .   1317:		}
         .          .   1318:		// XXX: should check that all pkg names are the same (else package is invalid)
         .          .   1319:		if pkgName == "" && strings.HasSuffix(fname, ".gno") {
    1.27GB     2.70GB   1320:			pkgName, err = PackageNameFromFileBody(fname, string(bz))
         .          .   1321:			if err != nil {
         .          .   1322:				return nil, err
         .          .   1323:			}
         .          .   1324:			if strings.HasSuffix(string(pkgName), "_test") {
         .          .   1325:				pkgName = pkgName[:len(pkgName)-len("_test")]
         .          .   1326:			}
         .          .   1327:		}
       2MB        2MB   1328:		memPkg.Files = append(memPkg.Files,
       6MB        6MB   1329:			&gnovm.MemFile{
         .          .   1330:				Name: fname,
    2.16GB     2.16GB   1331:				Body: string(bz),
         .          .   1332:			})
         .          .   1333:	}
         .          .   1334:
         .          .   1335:	// If no .gno files are present, package simply does not exist.
         .          .   1336:	if !memPkg.IsEmpty() {
         .        3MB   1337:		if err := validatePkgName(string(pkgName)); err != nil {
         .          .   1338:			return nil, err
         .          .   1339:		}
         .          .   1340:		memPkg.Name = string(pkgName)
         .          .   1341:	}
         .          .   1342:

if you squint close enough you'll notice a few things:

os.ReadFile(fpath) is unavoidable hence sure, we have to incur the 2.32GB allocation over time, but then when we go look down
when creating memPkg.Files, 2.16GB 2.16GB 1331: Body: string(bz), is another heavy handed allocation that is incurred from byteslice->string conversion yet in 1. we already paid the penalty but even more at no point do we mutate the body of each byteslice that was allocation previously.
and the above comes from this benchmark addition

diff --git a/gnovm/pkg/gnolang/bench_test.go b/gnovm/pkg/gnolang/bench_test.go
index b638ab66..82f00e65 100644
--- a/gnovm/pkg/gnolang/bench_test.go
+++ b/gnovm/pkg/gnolang/bench_test.go
@@ -1,6 +1,8 @@
 package gnolang
 
 import (
+       "path/filepath"
+       "runtime"
        "testing"
 )
 
@@ -29,3 +31,24 @@ func BenchmarkPkgIDFromPkgPath(b *testing.B) {
        }
        sink = nil
 }
+
+func BenchmarkReadMemPackage(b *testing.B) {
+       _, currentFile, _, _ := runtime.Caller(0)
+       rootOfRepo, err := filepath.Abs(filepath.Join(filepath.Dir(currentFile), "..", "..", ".."))
+       if err != nil {
+               b.Fatal(err)
+       }
+       demoDir := filepath.Join(rootOfRepo, "examples", "gno.land", "p", "demo")
+       ufmtDir := filepath.Join(demoDir, "ufmt")
+       b.ReportAllocs()
+       b.ResetTimer()
+
+       for i := 0; i < b.N; i++ {
+               sink = MustReadMemPackage(ufmtDir, "ufmt")
+       }
+
+       if sink == nil {
+               b.Fatal("Benchmark did not run!")
+       }
+       sink = nil
+}

Remedy

We should be very conservative and try to reuse memory as much in the VM, and in the case above where we don't modify that memory we should hook into the runtime unsafe conversion of byteslice header to string header then to string which will reuse the byteslices allocated by

Results

The result is a hefty reduction in all dimensions

$ benchstat before.txt after2.txt 
name              old time/op    new time/op    delta
ReadMemPackage-8     104µs ±13%      89µs ±17%  -15.05%  (p=0.001 n=10+9)

name              old alloc/op   new alloc/op   delta
ReadMemPackage-8    64.2kB ± 0%    22.1kB ± 0%  -65.56%  (p=0.000 n=10+10)

name              old allocs/op  new allocs/op  delta
ReadMemPackage-8      61.0 ± 0%      36.0 ± 0%  -40.98%  (p=0.000 n=10+10)

The text was updated successfully, but these errors were encountered:

…lices from os.ReadFile This change takes advantage of the fact that we can singly use the byteslices that were already allocated from os.ReadFile, given that the places that their string(bz) are invoked don't have any unsafe usages that then would modify those strings. This change shaves off considerable CPU and RAM and is in the spirit of having a fast VM that can be relied on and not stymied per se by garbage collection. With it, all the RAM contributions for: PackageNameFromFile and gnovm.MemFile disappear shaving off 4+GB and no longer even appearing as a direct RAM contributor in the top 10. ```shell $ benchstat before.txt after.txt name old time/op new time/op delta ReadMemPackage-8 101µs ± 8% 89µs ± 4% -11.54% (p=0.000 n=9+10) name old alloc/op new alloc/op delta ReadMemPackage-8 64.1kB ± 0% 34.2kB ± 0% -46.72% (p=0.001 n=8+9) name old allocs/op new allocs/op delta ReadMemPackage-8 59.0 ± 0% 56.0 ± 0% -5.08% (p=0.000 n=10+10) ``` and then now RAM usage profiles show ```shell Total: 7.41GB ROUTINE ======================== github.com/gnolang/gno/gnovm/pkg/gnolang.ReadMemPackageFromList in /Users/emmanuelodeke/go/src/github.com/gnolang/gno/gnovm/pkg/gnolang/nodes.go 34MB 6.98GB (flat, cum) 94.14% of Total . . 1310:func ReadMemPackageFromList(list []string, pkgPath string) (*gnovm.MemPackage, error) { 14.50MB 14.50MB 1311: memPkg := &gnovm.MemPackage{Path: pkgPath} . . 1312: var pkgName Name . . 1313: for _, fpath := range list { . . 1314: fname := filepath.Base(fpath) . 4.35GB 1315: bz, err := os.ReadFile(fpath) . . 1316: if err != nil { . . 1317: return nil, err . . 1318: } . . 1319: // XXX: should check that all pkg names are the same (else package is invalid) . . 1320: if pkgName == "" && strings.HasSuffix(fname, ".gno") { . . 1321: // The string derived from bz is never modified inside PackageNameFromFileBody . . 1322: // hence shave these unnecessary allocations by an unsafe . . 1323: // byteslice->string conversion per gnolang#3598 . 2.59GB 1324: pkgName, err = PackageNameFromFileBody(fname, unsafeBytesliceToStr(bz)) . . 1325: if err != nil { . . 1326: return nil, err . . 1327: } . . 1328: if strings.HasSuffix(string(pkgName), "_test") { . . 1329: pkgName = pkgName[:len(pkgName)-len("_test")] . . 1330: } . . 1331: } 6.50MB 6.50MB 1332: memPkg.Files = append(memPkg.Files, 13MB 13MB 1333: &gnovm.MemFile{ . . 1334: Name: fname, . . 1335: // The string derived from bz is never modified, hence to shave . . 1336: // unnecessary allocations, perform this unsafe byteslice->string . . 1337: // conversion per gnolang#3598 . . 1338: // and the Go garbage collector will knows to garbage collect . . 1339: // bz when no other object points to that memory. . . 1340: Body: unsafeBytesliceToStr(bz), . . 1341: }) . . 1342: } . . 1343: . . 1344: // If no .gno files are present, package simply does not exist. . . 1345: if !memPkg.IsEmpty() { . 7.01MB 1346: if err := validatePkgName(string(pkgName)); err != nil { . . 1347: return nil, err . . 1348: } . . 1349: memPkg.Name = string(pkgName) . . 1350: } ``` but previously looked like ```shell ROUTINE ======================== github.com/gnolang/gno/gnovm/pkg/gnolang.ReadMemPackageFromList in /Users/emmanuelodeke/go/src/github.com/gnolang/gno/gnovm/pkg/gnolang/nodes.go 5.55GB 11.46GB (flat, cum) 97.01% of Total . . 1309:func ReadMemPackageFromList(list []string, pkgPath string) (*gnovm.MemPackage, error) { 13MB 13MB 1310: memPkg := &gnovm.MemPackage{Path: pkgPath} . . 1311: var pkgName Name . . 1312: for _, fpath := range list { . . 1313: fname := filepath.Base(fpath) . 3.66GB 1314: bz, err := os.ReadFile(fpath) . . 1315: if err != nil { . . 1316: return nil, err . . 1317: } . . 1318: // XXX: should check that all pkg names are the same (else package is invalid) . . 1319: if pkgName == "" && strings.HasSuffix(fname, ".gno") { 2.02GB 4.27GB 1320: pkgName, err = PackageNameFromFileBody(fname, string(bz)) . . 1321: if err != nil { . . 1322: return nil, err . . 1323: } . . 1324: if strings.HasSuffix(string(pkgName), "_test") { . . 1325: pkgName = pkgName[:len(pkgName)-len("_test")] . . 1326: } . . 1327: } 4.50MB 4.50MB 1328: memPkg.Files = append(memPkg.Files, 9.50MB 9.50MB 1329: &gnovm.MemFile{ . . 1330: Name: fname, 3.50GB 3.50GB 1331: Body: string(bz), . . 1332: }) . . 1333: } . . 1334: . . 1335: // If no .gno files are present, package simply does not exist. . . 1336: if !memPkg.IsEmpty() { . 6.51MB 1337: if err := validatePkgName(string(pkgName)); err != nil { . . 1338: return nil, err . . 1339: } . . 1340: memPkg.Name = string(pkgName) . . 1341: } ``` Updates gnolang#3435 Fixes gnolang#3598

github-project-automation bot added this to 🧙‍♂️gno.land core team Jan 23, 2025

github-project-automation bot moved this to Triage in 🧙‍♂️gno.land core team Jan 23, 2025

odeke-em mentioned this issue Jan 23, 2025

gnovm/stdlibs/bytes: lots of time spent in runtime garbage collection than in running programs #3435

Open

odeke-em linked a pull request Jan 23, 2025 that will close this issue

perf(gnovm/pkg/gnolang): make ReadMemPackageFromList singly use byteslices from os.ReadFile #3599

Draft

kristovatlas added the non-security label Jan 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gnovm/pkg/gnolang: ReadMemPackage should reuse byteslices from reading files as those byteslices are not modified; without incurring expensive byteslice->string conversion #3598

gnovm/pkg/gnolang: ReadMemPackage should reuse byteslices from reading files as those byteslices are not modified; without incurring expensive byteslice->string conversion #3598

odeke-em commented Jan 23, 2025

gnovm/pkg/gnolang: ReadMemPackage should reuse byteslices from reading files as those byteslices are not modified; without incurring expensive byteslice->string conversion #3598

gnovm/pkg/gnolang: ReadMemPackage should reuse byteslices from reading files as those byteslices are not modified; without incurring expensive byteslice->string conversion #3598

Comments

odeke-em commented Jan 23, 2025

Remedy

Results