Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Map key sorting could be improved #1

Open
emidoots opened this issue Dec 30, 2020 · 0 comments
Open

Map key sorting could be improved #1

emidoots opened this issue Dec 30, 2020 · 0 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@emidoots
Copy link
Member

Map keys are sorted according to regular Go rules, but a number of Go values cannot be sorted according to those rules. As it stands, valast will produce unstable output in such situations.

It would be nice to find a better way to resolve this. Feedback on how to handle this very welcome.

  • Panic or error when such a map key is encountered, ask the user provide a comparison function
  • See if go-cmp does anything for this - I suspect they don't have to because they can just compare map keys for equality
  • Take into consideration the drawbacks and benefits with the go-spew approach, output not being sorted by default, sorting by spewed keys is interesting but may have edge cases that are not handled, etc.
@emidoots emidoots added enhancement New feature or request help wanted Extra attention is needed labels Dec 30, 2020
emidoots pushed a commit that referenced this issue Dec 30, 2020
add stable sorting of map keys
fc56d3f 
Creates issue #1

Signed-off-by: Stephen Gutekanst <stephen@hexops.com>
emidoots pushed a commit that referenced this issue Feb 18, 2021
Add type expression cache (66% performance improvement)
2577a24 
With complex data types, this can provide a 66% performance improvement.

Before:

```
$ go test -bench=.
goos: darwin
goarch: amd64
pkg: github.com/hexops/valast
BenchmarkComplexType-16    	       2	 564812395 ns/op
PASS
ok  	github.com/hexops/valast	9.372s
```

After:

```
$ go test -bench=.
goos: darwin
goarch: amd64
pkg: github.com/hexops/valast
BenchmarkComplexType-16    	       6	 189646854 ns/op
PASS
ok  	github.com/hexops/valast	6.379s
```

In practice this can result in tests running much faster, before:

```
--- PASS: TestUnionMerge (6.38s)
    --- PASS: TestUnionMerge/#00 (1.42s)
    --- PASS: TestUnionMerge/#1 (1.17s)
    --- PASS: TestUnionMerge/#2 (2.32s)
    --- PASS: TestUnionMerge/#3 (0.00s)
    --- PASS: TestUnionMerge/#4 (0.00s)
    --- PASS: TestUnionMerge/#5 (1.46s)
```

After:

```
--- PASS: TestUnionMerge (2.96s)
    --- PASS: TestUnionMerge/#00 (1.05s)
    --- PASS: TestUnionMerge/#1 (0.77s)
    --- PASS: TestUnionMerge/#2 (0.77s)
    --- PASS: TestUnionMerge/#3 (0.00s)
    --- PASS: TestUnionMerge/#4 (0.00s)
    --- PASS: TestUnionMerge/#5 (0.37s)
```

Also appears to fix hexops/autogold#16

Signed-off-by: Stephen Gutekanst <stephen@hexops.com>
emidoots pushed a commit that referenced this issue Feb 18, 2021
Add type expression cache (66% performance improvement) (#10)
6fff268 
With complex data types, this can provide a 66% performance improvement.

Before:

```
$ go test -bench=.
goos: darwin
goarch: amd64
pkg: github.com/hexops/valast
BenchmarkComplexType-16    	       2	 564812395 ns/op
PASS
ok  	github.com/hexops/valast	9.372s
```

After:

```
$ go test -bench=.
goos: darwin
goarch: amd64
pkg: github.com/hexops/valast
BenchmarkComplexType-16    	       6	 189646854 ns/op
PASS
ok  	github.com/hexops/valast	6.379s
```

In practice this can result in tests running much faster, before:

```
--- PASS: TestUnionMerge (6.38s)
    --- PASS: TestUnionMerge/#00 (1.42s)
    --- PASS: TestUnionMerge/#1 (1.17s)
    --- PASS: TestUnionMerge/#2 (2.32s)
    --- PASS: TestUnionMerge/#3 (0.00s)
    --- PASS: TestUnionMerge/#4 (0.00s)
    --- PASS: TestUnionMerge/#5 (1.46s)
```

After:

```
--- PASS: TestUnionMerge (2.96s)
    --- PASS: TestUnionMerge/#00 (1.05s)
    --- PASS: TestUnionMerge/#1 (0.77s)
    --- PASS: TestUnionMerge/#2 (0.77s)
    --- PASS: TestUnionMerge/#3 (0.00s)
    --- PASS: TestUnionMerge/#4 (0.00s)
    --- PASS: TestUnionMerge/#5 (0.37s)
```

Also appears to fix hexops/autogold#16

Signed-off-by: Stephen Gutekanst <stephen@hexops.com>
emidoots pushed a commit that referenced this issue Feb 19, 2025
@bobheadxi @unknwon
customtype: add ability to register any custom type (#27)
41f59f9 
* customtype: add ability to register any custom type

* make customtype internal, add valast.RegisterType

* make reflect.Array support custom type (#1)

---------

Co-authored-by: Joe Chen <jc@unknwon.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@emidoots