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.

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

Remove LatestMap, to reduce memory allocation #2351

Merged
merged 2 commits into from
Mar 21, 2017
Merged

Conversation

bboreham
Copy link
Collaborator

@bboreham bboreham commented Mar 20, 2017

Previously, classes like StringLatestMap were implemented as a shim over LatestMap.
This meant that the memory allocations were:
[ps.Map leaf] -> [interface{} indirection] -> LatestEntry -> [interface{} indirection] -> [string contents]

Removing LatestMap and storing the entry via pointer rather than value gives us instead:

[ps.Map leaf] -> stringLatestEntry -> [string contents]

Also the LatestEntry was copied one more time on entry into the map. Thus this PR removes 50% of the memory allocations needed to create this particular structure, per entry.

(not counting the allocation for the map key)

Structs like StringLatestMap now use ps.Map directly, which saves
a memory allocation for LatestEntry.Value to point to.
The values in the ps.Map are now pointers, which saves a memory
allocation indirecting a value type to an interface{}
@bboreham
Copy link
Collaborator Author

bboreham commented Mar 20, 2017

Some stats (run on a slightly different commit):

Running the following test:

package report                                                                                                                                                        
                                                                                                                                                                      
import "github.com/ugorji/go/codec"                                                                                                                                   
import "io/ioutil"                                                                                                                                                    
import "testing"

func TestDecode(t *testing.T) {
    b, _ := ioutil.ReadFile("fixed.msgpack")
    for i := 0; i < 100; i++ {
        r := Report{}
        handler := &codec.MsgpackHandle{}
        d := codec.NewDecoderBytes(b, handler)
        d.Decode(&r)
    }
}

where fixed.msgpack is a 1.5MB file supplied by Adam;

On master, commit d64d66e:

$ go test -run TestDecode
ok  	github.com/weaveworks/scope/report	1.935s
ok  	github.com/weaveworks/scope/report	1.989s
ok  	github.com/weaveworks/scope/report	1.937s
ok  	github.com/weaveworks/scope/report	2.065s

With this PR, commit de97a72:

PASS
ok  	github.com/weaveworks/scope/report	1.678s
ok  	github.com/weaveworks/scope/report	1.652s
ok  	github.com/weaveworks/scope/report	1.756s
ok  	github.com/weaveworks/scope/report	1.743s

That's about a 15% speed-up, fastest to fastest.

@2opremio
Copy link
Contributor

Did you find generate_latest_map useful?

@2opremio
Copy link
Contributor

2opremio commented Mar 21, 2017

(i.e. we don't really need to maintain it, so I hope it was helpful and you didn't update it because it was there)

@bboreham
Copy link
Collaborator Author

Yes, it's easier to edit the code once and generate twice, than to edit it twice.

@bboreham bboreham merged commit 13fc9f7 into master Mar 21, 2017
@rade rade deleted the remove-LatestMap branch July 5, 2017 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants