From ce94db1fdd28bd8071c2800bd1ed718e24f3d607 Mon Sep 17 00:00:00 2001 From: Eric Myhre Date: Fri, 27 Mar 2020 12:58:01 +0100 Subject: [PATCH 1/2] Expose styles with nicer syntax. I'm not sure if there's a name for this way of grouping methods, but the end result is dotted.Access.Patterns, and it's kinda nice. This was extensively discussed in the PR: https://github.com/ipld/go-ipld-prime/pull/49#issuecomment-596465234 https://github.com/ipld/go-ipld-prime/pull/49#issuecomment-597549108 https://github.com/ipld/go-ipld-prime/pull/49#issuecomment-604412736 The "inlinability_test.go" file can be compiled with special flags (described in the comment at the top of the file) to see the outcome in assembly. Result? Yep, things are still inlinable; this change is performance neutral. --- node/basic/HACKME.md | 43 ++++++++++++++++---- node/basic/inlinability/inlinability_test.go | 22 ++++++++++ node/basic/style.go | 26 ++++++++++++ 3 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 node/basic/inlinability/inlinability_test.go create mode 100644 node/basic/style.go diff --git a/node/basic/HACKME.md b/node/basic/HACKME.md index 7f1c4bc9..fa090f9f 100644 --- a/node/basic/HACKME.md +++ b/node/basic/HACKME.md @@ -105,14 +105,41 @@ Note that these remarks are for the `basicnode` package, but may also apply to other implementations too (e.g., our codegen output follows similar overall logic). -### nodestyles are implemented as exported concrete types +### nodestyles are available through a singleton -This is sorta arbitrary. We could equally easily make them exported as -package scope vars (which happen to be singletons), or as functions -(which happen to have fixed returns). +Every NodeStyle available from this package is exposed as a field +in a struct of which there's one public exported instance available, +called 'Style'. -These choices affect syntax, but not really semantics. +This means you can use it like this: -In any of these cases, we'd still end up with concrete types per style -(so that there's a place to hang the methods)... so, it seemed simplest -to just export them. +```go +nbm := basicnode.Style.Map.NewBuilder() +nbs := basicnode.Style.String.NewBuilder() +nba := basicnode.Style.Any.NewBuilder() +// etc +``` + +(If you're interested in the performance of this: it's free! +Methods called at the end of the chain are inlinable. +Since all of the types of the structures on the way there are zero-member +structs, the compiler can effectively treat them as constants, +and thus freely elide any memory dereferences that would +otherwise be necessary to get methods on such a value.) + +### nodestyles are (also) available as exported concrete types + +The 'Style' singleton is one way to access the NodeStyle in this package; +their exported types are another equivalent way. + +```go +basicnode.Style.Map = basicnode.Style__Map{} +``` + +It is recommended to use the singleton style; +they compile to identical assembly, and the singleton is syntactically prettier. + +We may make these concrete types unexported in the future. +A decision on this is deferred until some time has passed and +we can accumulate reasonable certainty that there's no need for an exported type +(such as type assertions, etc). diff --git a/node/basic/inlinability/inlinability_test.go b/node/basic/inlinability/inlinability_test.go new file mode 100644 index 00000000..a73b2462 --- /dev/null +++ b/node/basic/inlinability/inlinability_test.go @@ -0,0 +1,22 @@ +// Compile with '-gcflags -S' and grep the assembly for `"".Test`. +// You'll find that both methods produce identical bodies modulo line numbers. +package inlinability + +import ( + "testing" + + ipld "github.com/ipld/go-ipld-prime" + basicnode "github.com/ipld/go-ipld-prime/node/basic" +) + +var sink ipld.NodeBuilder + +func TestStructReference(t *testing.T) { + nb := basicnode.Style__String{}.NewBuilder() + sink = nb +} + +func TestVarReference(t *testing.T) { + nb := basicnode.Style.String.NewBuilder() + sink = nb +} diff --git a/node/basic/style.go b/node/basic/style.go new file mode 100644 index 00000000..a5b87f55 --- /dev/null +++ b/node/basic/style.go @@ -0,0 +1,26 @@ +package basicnode + +// Style embeds a NodeStyle for every kind of Node implementation in this package. +// You can use it like this: +// +// basicnode.Style.Map.NewBuilder().BeginMap() //... +// +// and: +// +// basicnode.Style.String.NewBuilder().AssignString("x") // ... +// +// Most of the styles here are for one particular Kind of node (e.g. string, int, etc); +// you can use the "Any" style if you want a builder that can accept any kind of data. +var Style style + +type style struct { + Any Style__Any + Map Style__Map + List Style__List + Bool Style__Bool + Int Style__Int + Float Style__Float + String Style__String + Bytes Style__Bytes + Link Style__Link +} From 8a3e80f794864d4a6137dbc411ffec36ebd401e8 Mon Sep 17 00:00:00 2001 From: Eric Myhre Date: Fri, 27 Mar 2020 13:03:23 +0100 Subject: [PATCH 2/2] Discard inlinability check. This was only useful as a design research expedition. It's here in history if you want to refer to it. --- node/basic/inlinability/inlinability_test.go | 22 -------------------- 1 file changed, 22 deletions(-) delete mode 100644 node/basic/inlinability/inlinability_test.go diff --git a/node/basic/inlinability/inlinability_test.go b/node/basic/inlinability/inlinability_test.go deleted file mode 100644 index a73b2462..00000000 --- a/node/basic/inlinability/inlinability_test.go +++ /dev/null @@ -1,22 +0,0 @@ -// Compile with '-gcflags -S' and grep the assembly for `"".Test`. -// You'll find that both methods produce identical bodies modulo line numbers. -package inlinability - -import ( - "testing" - - ipld "github.com/ipld/go-ipld-prime" - basicnode "github.com/ipld/go-ipld-prime/node/basic" -) - -var sink ipld.NodeBuilder - -func TestStructReference(t *testing.T) { - nb := basicnode.Style__String{}.NewBuilder() - sink = nb -} - -func TestVarReference(t *testing.T) { - nb := basicnode.Style.String.NewBuilder() - sink = nb -}