-
Notifications
You must be signed in to change notification settings - Fork 534
plumbing: cache, enforce the use of cache in packfile decoder #698
Conversation
Decoder object can make use of an object cache to speed up processing. Previously the only way to specify it was changing manually the struct generated by NewDecodeForFile. This lead to some instances to be created without it and penalized performance. Now the cache should be explicitly passed to the constructor function. NewDecoder now creates objects with a cache using the default size. A new helper function was added to create cache objects with the default size as this becomes a common task now: cache.NewObjectLRUDefault() Signed-off-by: Javi Fontan <[email protected]>
plumbing/format/packfile/decoder.go
Outdated
// deserialization of all the objects | ||
// deserialization of all the objects. | ||
// | ||
// cacheObject is an ObjectLRU that is used to speed up the process. If cache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cache instance can be any implementation of cache.Object
plumbing/format/packfile/decoder.go
Outdated
// | ||
// cacheObject is an ObjectLRU that is used to speed up the process. If cache | ||
// is not needed you can pass nil. To create a cache object with the default | ||
// size you an use the helper cache.ObjectLRUDefault(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/an/can/
plumbing/format/packfile/decoder.go
Outdated
o: o, | ||
s: s, | ||
o: o, | ||
DeltaBaseCache: cacheObject, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you make DeltaBaseCache private?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've addressed these issues in the following commits.
Signed-off-by: Javi Fontan <[email protected]>
Signed-off-by: Javi Fontan <[email protected]>
@@ -24,6 +24,11 @@ func NewObjectLRU(maxSize FileSize) *ObjectLRU { | |||
return &ObjectLRU{MaxSize: maxSize} | |||
} | |||
|
|||
// NewObjectLRUDefault creates a new ObjectLRU with the default cache size. | |||
func NewObjectLRUDefault() *ObjectLRU { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no tested function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now cache tests are done twice. One with the previous cache and another time with a cache created with NewObjectLRUDefault
.
Signed-off-by: Javi Fontan <[email protected]>
Decoder object can make use of an object cache to speed up processing.
Previously the only way to specify it was changing manually the struct
generated by NewDecodeForFile. This lead to some instances to be created
without it and penalized performance.
Now the cache should be explicitly passed to the constructor function.
NewDecoder now creates objects with a cache using the default size.
A new helper function was added to create cache objects with the default
size as this becomes a common task now: