Skip to content

feat: load packages lazily #3669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions gno.land/pkg/integration/testdata/addpkg_outofgas.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@ gnoland start
gnokey maketx addpkg -pkgdir $WORK/foo -pkgpath gno.land/r/foo -gas-fee 1000000ugnot -gas-wanted 220000 -broadcast -chainid=tendermint_test test1


# add bar package - out of gas at store.GetPackage() with gas 60000
# add bar package - out of gas at store.GetPackage() with gas 70000

! gnokey maketx addpkg -pkgdir $WORK/bar -pkgpath gno.land/r/bar -gas-fee 1000000ugnot -gas-wanted 60000 -broadcast -chainid=tendermint_test test1
! gnokey maketx addpkg -pkgdir $WORK/bar -pkgpath gno.land/r/bar -gas-fee 1000000ugnot -gas-wanted 70000 -broadcast -chainid=tendermint_test test1

stderr '--= Error =--'
stderr 'Data: out of gas error'
stderr '--= /Error =--'



# out of gas at store.store.GetTypeSafe() with gas 63000
# out of gas at store.store.GetTypeSafe() with gas 73000

! gnokey maketx addpkg -pkgdir $WORK/bar -pkgpath gno.land/r/bar -gas-fee 1000000ugnot -gas-wanted 63000 -broadcast -chainid=tendermint_test test1
! gnokey maketx addpkg -pkgdir $WORK/bar -pkgpath gno.land/r/bar -gas-fee 1000000ugnot -gas-wanted 73000 -broadcast -chainid=tendermint_test test1

stderr '--= Error =--'
stderr 'Data: out of gas error'
Expand Down
4 changes: 2 additions & 2 deletions gno.land/pkg/integration/testdata/restart.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
loadpkg gno.land/r/demo/counter $WORK
gnoland start

gnokey maketx call -pkgpath gno.land/r/demo/counter -func Incr -gas-fee 1000000ugnot -gas-wanted 200000 -broadcast -chainid tendermint_test test1
gnokey maketx call -pkgpath gno.land/r/demo/counter -func Incr -gas-fee 2000000ugnot -gas-wanted 350000 -broadcast -chainid tendermint_test test1
stdout '\(1 int\)'

gnoland restart

gnokey maketx call -pkgpath gno.land/r/demo/counter -func Incr -gas-fee 1000000ugnot -gas-wanted 200000 -broadcast -chainid tendermint_test test1
gnokey maketx call -pkgpath gno.land/r/demo/counter -func Incr -gas-fee 2000000ugnot -gas-wanted 350000 -broadcast -chainid tendermint_test test1
stdout '\(2 int\)'

-- counter.gno --
Expand Down
4 changes: 2 additions & 2 deletions gno.land/pkg/integration/testdata/simulate_gas.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ loadpkg gno.land/r/simulate $WORK/simulate
gnoland start

# simulate only
gnokey maketx call -pkgpath gno.land/r/simulate -func Hello -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test -simulate only test1
gnokey maketx call -pkgpath gno.land/r/simulate -func Hello -gas-fee 2000000ugnot -gas-wanted 2200000 -broadcast -chainid=tendermint_test -simulate only test1
stdout 'GAS USED: 99371'

# simulate skip
gnokey maketx call -pkgpath gno.land/r/simulate -func Hello -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test -simulate skip test1
gnokey maketx call -pkgpath gno.land/r/simulate -func Hello -gas-fee 2000000ugnot -gas-wanted 2200000 -broadcast -chainid=tendermint_test -simulate skip test1
stdout 'GAS USED: 99371' # same as simulate only


Expand Down
50 changes: 40 additions & 10 deletions gnovm/pkg/gnolang/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,35 +224,65 @@
//----------------------------------------
// top level Run* methods.

// Upon restart, preprocess all MemPackage and save blocknodes.
// This is a temporary measure until we optimize/make-lazy.
// PreprocessAllFilesAndSaveBlockNodes prepares the store for lazy loading
// by registering packages in the store but not fully preprocessing them.
//
// NOTE: package paths not beginning with gno.land will be allowed to override,
// to support cases of stdlibs processed through [RunMemPackagesWithOverrides].
func (m *Machine) PreprocessAllFilesAndSaveBlockNodes() {
ch := m.Store.IterMemPackage()
for memPkg := range ch {
// Set up a PackageGetter that will do the actual preprocessing when needed
pkgGetter := func(pkgPath string, store Store) (*PackageNode, *PackageValue) {
// Get the original MemPackage
memPkg := store.GetMemPackage(pkgPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick note: the problem with this is that nodes which are restarted have a different set of packages in the cache compared to ones which have been running with a cache; consequently, loading a DB from the store needs to happen with a clean gas meter.

I would also suggest that the best solution is one that keeps packages in the store using a cache, so that the amount of packages kept in the store is actually bounded to a specific amount

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking about alternative implementations because the one in the PR seems to have issues with persistence. I'll keep this in mind whether i continue with this or go in a different direction.

if memPkg == nil {
return nil, nil
}

Check warning on line 239 in gnovm/pkg/gnolang/machine.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/machine.go#L233-L239

Added lines #L233 - L239 were not covered by tests

// Do the actual preprocessing when package is requested
fset := ParseMemPackage(memPkg)
pn := NewPackageNode(Name(memPkg.Name), memPkg.Path, fset)
m.Store.SetBlockNode(pn)
PredefineFileSet(m.Store, pn, fset)
store.SetBlockNode(pn)
PredefineFileSet(store, pn, fset)

Check warning on line 246 in gnovm/pkg/gnolang/machine.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/machine.go#L244-L246

Added lines #L244 - L246 were not covered by tests
for _, fn := range fset.Files {
// Save Types to m.Store (while preprocessing).
fn = Preprocess(m.Store, pn, fn).(*FileNode)
// Save BlockNodes to m.Store.
SaveBlockNodes(m.Store, fn)
// Save Types to store (while preprocessing)
fn = Preprocess(store, pn, fn).(*FileNode)
// Save BlockNodes to store
SaveBlockNodes(store, fn)

Check warning on line 251 in gnovm/pkg/gnolang/machine.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/machine.go#L248-L251

Added lines #L248 - L251 were not covered by tests
}

// Normally, the fileset would be added onto the
// package node only after runFiles(), but we cannot
// run files upon restart (only preprocess them).
// So, add them here instead.
// TODO: is this right?

if pn.FileSet == nil {
pn.FileSet = fset
} else {
// This happens for non-realm file tests.
// TODO ensure the files are the same.
}

pv := pn.NewPackage()
store.SetObject(pv)
store.SetCachePackage(pv)
store.SetBlockNode(pn)

return pn, pv

Check warning on line 272 in gnovm/pkg/gnolang/machine.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/machine.go#L267-L272

Added lines #L267 - L272 were not covered by tests
}

// Register the PackageGetter with the store
m.Store.SetPackageGetter(pkgGetter)

// Register unprocessed packages in store
ch := m.Store.IterMemPackage()
for memPkg := range ch {
if memPkg == nil {
continue

Check warning on line 282 in gnovm/pkg/gnolang/machine.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/machine.go#L276-L282

Added lines #L276 - L282 were not covered by tests
}
// Just add the package to the store without preprocessing
m.Store.AddMemPackage(memPkg)

Check warning on line 285 in gnovm/pkg/gnolang/machine.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/machine.go#L285

Added line #L285 was not covered by tests
}
}

Expand Down
Loading