gonb deletes all "//go:noinline" comments before run

[alex91823]

package benchmark_test

import "testing"

func Benchmark_Cache_ADD_A(b *testing.B) {
	f := func() {
		db := NewDataBase()

		Fn(db)
	}

	for range b.N {
		f()
	}
}

type Cache interface {
	Add(key, value string)
	Get(key string) (string, bool)
}

type DataBase struct {
	m map[string]string
}

func NewDataBase() *DataBase {
	return &DataBase{m: make(map[string]string)}
}

func (b *DataBase) Add(key, value string) {
	b.m[key] = value
}

func (b *DataBase) Get(key string) (string, bool) {
	value, ok := b.m[key]

	return value, ok
}

//go:noinline
func Fn(c Cache) {
	c.Add("key", "42")
}

func main() {
	dataBase := NewDataBase()

	Fn(dataBase)
}

%test -test.bench=. -test.run=Benchmark. -test.benchmem

Benchmark_Cache_ADD_A-16 87825375 14.22 ns/op 0 B/op 0 allocs/op

The result is wrong because gonb delete all //go:noinline comments in code before run.

Right result is:

Benchmark_Cache_ADD-16 5417229 206.5 ns/op 344 B/op 3 allocs/op

Please don't delete //go:noinline comments...

[janpfeifer]

hi @alex91823 ,

Sorry about the issue, I hadn't thought about that ...

Let me give you some context how GoNB works: whenever a cell is "executed", GoNB parses it (using the Go parser), takes the top level declarations (functions, variables, types, constants, imports), and merges them to its collection of such declarations.

Then it dynamically generates a main.go file on a temporary directory, along with a go.mod. It does a go imports, go get and execute the results.

As you already noticed, the parser didn't include the comments along the top level declarations (although comments inside them are preserved, since gonb saves the whole body of the function/var/constant/etc.). So it's not that gonb "deletes" the comment, it's more like it never saw them. The net result is the same though.

A quick way to see what you get is just to print the generated main.go program is just to cat it. In a cell the special !* prefix will execute the shell command that follows in the temporary directory with the code (as opposed to the working directory, usually where the notebook is saved). So:

[janpfeifer]

(sry, I submitted the previous comment before finishing it)

All that just to say that it's not necessarily a trivial fix -- I have no idea if the parser provides the comments associated with a function easily.

But I'll try to take a stab at it and report back here.

[janpfeifer]

Ok, it was not as simple as I hoped it to be, but it was also not as bad as I feared it could have been. See #151

It is submitted to the head in the main branch, and passes all the tests -- I included a line //go:noinline in the test, so it will be covered.

Would you like to give it a shot ? You'll have to clone the repository in your box, and manually install it from there: from the cloned directory, do a go run . --install (add any other flags you may want), and then restart Jupyter (if you haven't changed the installation flags, you can simply restart the kernel, with 0, 0 keys in Jupyter)

You can quickly test it with a couple of cells:

// f sums 2 numbers
//go:noinline
func f(a, b int) int {  // Some comment
    return a + b
}

%%
fmt.Printf("1+2=%d\n", f(1, 1))

And to verify it generated the right code:

!*cat main.go

You should get:

package main

import (
	"flag"
	"fmt"
)

// F sums 2 numbers
//go:noinline
func f(a, b int) int {  // Some comment
    return a + b
}


func main() {
	flag.Parse()
fmt.Printf("1+2=%d\n", f(1, 1))

}

If all goes well for you, I'll create a new release.

[alex91823]

@janpfeifer Thank you. I have tested! All works correct.

[janpfeifer]

Awesome. I released gonb v0.10.7, so anyone can get it.

Unfortunately thought, go.lsp.dev seems down, so I'm not able to build a new pre-built Docker. But I've contacted the author, so hopefully this will be fixed later and there will be an updated docker later.