Skip to content

Commit

Permalink
Change flag storage to reduce memory use (#108)
Browse files Browse the repository at this point in the history
  • Loading branch information
FollowTheProcess authored Oct 5, 2024
1 parent e355191 commit 842c856
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 23 deletions.
35 changes: 20 additions & 15 deletions internal/flag/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,21 @@ import (

// Set is a set of command line flags.
type Set struct {
// TODO(@FollowTheProcess): Figure out a way so that we don't have to store 2 maps
// as it's very memory wasteful
flags map[string]Entry // The actual stored flags, can lookup by name
shorthands map[rune]Entry // The flags by shorthand
args []string // Arguments minus flags or flag values
extra []string // Arguments after "--" was hit
// Note: flags and shorthands are different "views" to the same *Entry, the Entry
// is not duplicated, it's just two maps to the same pointer so we can look up
// using either

flags map[string]*Entry // The actual stored flags, can lookup by name
shorthands map[rune]*Entry // The flags by shorthand
args []string // Arguments minus flags or flag values
extra []string // Arguments after "--" was hit
}

// NewSet builds and returns a new set of flags.
func NewSet() *Set {
return &Set{
flags: make(map[string]Entry),
shorthands: make(map[rune]Entry),
flags: make(map[string]*Entry),
shorthands: make(map[rune]*Entry),
}
}

Expand Down Expand Up @@ -56,7 +58,7 @@ func AddToSet[T Flaggable](set *Set, flag Flag[T]) error {
defaultValueNoArg = "1"
}

entry := Entry{
entry := &Entry{
Value: flag,
Name: flag.name,
Usage: flag.usage,
Expand All @@ -76,26 +78,26 @@ func AddToSet[T Flaggable](set *Set, flag Flag[T]) error {

// Get gets a flag [Entry] from the Set by name and a boolean to indicate
// whether it was present.
func (s *Set) Get(name string) (Entry, bool) {
func (s *Set) Get(name string) (*Entry, bool) {
if s == nil {
return Entry{}, false
return nil, false
}
entry, ok := s.flags[name]
if !ok {
return Entry{}, false
return nil, false
}
return entry, true
}

// GetShort gets a flag [Entry] from the Set by it's shorthand and a boolean to indicate
// whether it was present.
func (s *Set) GetShort(short rune) (Entry, bool) {
func (s *Set) GetShort(short rune) (*Entry, bool) {
if s == nil {
return Entry{}, false
return nil, false
}
entry, ok := s.shorthands[short]
if !ok {
return Entry{}, false
return nil, false
}
return entry, true
}
Expand Down Expand Up @@ -206,6 +208,9 @@ func (s *Set) Usage() (string, error) {

for _, name := range names {
entry := s.flags[name]
if entry == nil {
return "", fmt.Errorf("*Entry stored against key %s was nil", name) // Should never happen
}
var shorthand string
if entry.Shorthand != NoShortHand {
shorthand = fmt.Sprintf("-%s", string(entry.Shorthand))
Expand Down
18 changes: 10 additions & 8 deletions internal/flag/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestParse(t *testing.T) {
t.Helper()
f, exists := set.Get("something")
test.False(t, exists)
test.Equal(t, f, flag.Entry{})
test.Equal(t, f, nil)

test.EqualFunc(t, set.Args(), nil, slices.Equal)
},
Expand All @@ -53,7 +53,7 @@ func TestParse(t *testing.T) {
t.Helper()
f, exists := set.Get("something")
test.False(t, exists)
test.Equal(t, f, flag.Entry{})
test.Equal(t, f, nil)

test.EqualFunc(t, set.Args(), []string{"some", "args", "here", "no", "flags"}, slices.Equal)
},
Expand All @@ -70,7 +70,7 @@ func TestParse(t *testing.T) {
t.Helper()
f, exists := set.Get("something")
test.False(t, exists)
test.Equal(t, f, flag.Entry{})
test.Equal(t, f, nil)

test.EqualFunc(t, set.Args(), []string{"some", "args", "here", "no", "flags", "extra", "args"}, slices.Equal)
test.EqualFunc(t, set.ExtraArgs(), []string{"extra", "args"}, slices.Equal)
Expand All @@ -88,7 +88,7 @@ func TestParse(t *testing.T) {
t.Helper()
f, exists := set.Get("flag")
test.False(t, exists)
test.Equal(t, f, flag.Entry{})
test.Equal(t, f, nil)

test.EqualFunc(t, set.Args(), []string{"some", "args", "here"}, slices.Equal)
},
Expand Down Expand Up @@ -827,6 +827,7 @@ func TestParse(t *testing.T) {
// Get by short
entry, exists = set.GetShort('c')
test.False(t, exists) // Short shouldn't exist
test.Equal(t, entry, nil)
},
args: []string{"--count", "1"},
wantErr: false,
Expand Down Expand Up @@ -856,6 +857,7 @@ func TestParse(t *testing.T) {
// Get by short
entry, exists = set.GetShort('c')
test.False(t, exists) // Short shouldn't exist
test.Equal(t, entry, nil)
},
args: []string{"-c", "1"},
wantErr: true,
Expand Down Expand Up @@ -968,7 +970,7 @@ func TestFlagSet(t *testing.T) {
t.Helper()
f, exists := set.Get("missing")
test.False(t, exists)
test.Equal(t, f, flag.Entry{})
test.Equal(t, f, nil)
},
},
{
Expand All @@ -981,7 +983,7 @@ func TestFlagSet(t *testing.T) {
t.Helper()
f, exists := set.GetShort('d')
test.False(t, exists)
test.Equal(t, f, flag.Entry{})
test.Equal(t, f, nil)
},
},
{
Expand All @@ -994,7 +996,7 @@ func TestFlagSet(t *testing.T) {
t.Helper()
f, exists := set.Get("missing")
test.False(t, exists)
test.Equal(t, f, flag.Entry{})
test.Equal(t, f, nil)
},
},
{
Expand All @@ -1007,7 +1009,7 @@ func TestFlagSet(t *testing.T) {
t.Helper()
f, exists := set.GetShort('m')
test.False(t, exists)
test.Equal(t, f, flag.Entry{})
test.Equal(t, f, nil)
},
},
{
Expand Down

0 comments on commit 842c856

Please sign in to comment.