Fix some provider subsystem performance issues #319
Conversation
ghost
assigned 
Stebalien
force-pushed
the
fix/provider-mayhem
branch
2 times, most recently
from
84b9fac
to
3ab4992
Compare
Stebalien
force-pushed
the
fix/provider-mayhem
branch
from
3ab4992
to
c095545
Compare
|
Note: We should be doing GC in a separate thread but it's a little tricky to get this right without race conditions. We can save this for a follow-up PR. |
Stebalien
force-pushed
the
fix/provider-mayhem
branch
from
c095545
to
7ce5021
Compare
| @@ -30,7 +30,7 @@ var defaultCleanupInterval = time.Hour | |||
| type ProviderManager struct { | |||
| // all non channel fields are meant to be accessed only within | |||
| // the run method | |||
| providers *lru.Cache | |||
| providers *lru.LRU | |||
There was a problem hiding this comment.
what's the difference between these two?
There was a problem hiding this comment.
Are we sure this is safe? It could come back to bite us.
There was a problem hiding this comment.
Yes. The old provider set wasn't safe to use from multiple threads anyways as we'd modify the *providerSet structs.
| provs.setVal(p, now) | ||
| if provs, ok := pm.providers.Get(k); ok { | ||
| provs.(*providerSet).setVal(p, now) | ||
| } // else not cached, just write through |
| data, ok := i.([]byte) | ||
| if !ok { | ||
| return time.Time{}, fmt.Errorf("data was not a []byte") | ||
| func readTimeValue(data []byte) (time.Time, error) { |
There was a problem hiding this comment.
avoiding the interface{} is a nice little gain! No more allocation for passing byte slices.
There was a problem hiding this comment.
(this was left over from the datastore refactor when we replaced all the interfaces with []byte)
| fallthrough | ||
| case now.Sub(t) > ProvideValidity: | ||
| // or just expired | ||
| err = pm.dstore.Delete(ds.RawKey(e.Key)) |
There was a problem hiding this comment.
shouldn't we also clean the lru cache here? Alternatively we can simply flush it completely on gc.
There was a problem hiding this comment.
Ah, there is a Purge call before entering gc, good.
| // drop them. | ||
| // | ||
| // Much faster than GCing. | ||
| pm.providers.Purge() |
|
Only concern here is removing the thread-safety from the LRU cache. |
This is just extra work as we write through anyways.
We only access it from a single goroutine.
1. Don't be n^2. 2. Don't bother walking the cache, just drop it.
batches deletes
Stebalien
force-pushed
the
fix/provider-mayhem
branch
from
6f26a7a
to
7bdc7a5
Compare
Avoid populating the cache when garbage collecting (this just destroys the cache and hurts performance).Just drop the cache.keys()returns the CID once per provider (and we then iterate over all providers once per provider).related: #316