[Fix] Part-1 - LevelDbStore#3414
Conversation
vncoelho
left a comment
There was a problem hiding this comment.
can this PR be tested without PART 2?
Yes |
|
Hey Chris, can you please add a few UTs to demonstrate how your pr improves the leveldb? I am reviewing your code, but dont really clear how and where the improve works. |
|
|
Will check part 2 as well then, but please avoid doing this, cause it makes a single pr not reviewable. |
Just trying to make it small. |
|
Added |
|
@superboyiii can you please rake a look at this pr and do some tests on it? |
Sure |
|
@cschuchardt88 Seems not an issue from this PR because it occurs on master in block 2724677, I will open an issue. |
| if (disposing) | ||
| { | ||
| FreeManagedObjects(); | ||
| } | ||
| if (Handle != nint.Zero) | ||
| { | ||
| FreeUnManagedObjects(); | ||
| Handle = nint.Zero; | ||
| } | ||
| _disposed = true; |
There was a problem hiding this comment.
I prefer to use Interlocked for thread safe flag use
There was a problem hiding this comment.
That's reasonable I'll add it in when I get a chance
There was a problem hiding this comment.
Looks like you can't Interlocked. its a non-ref returning type. Reason for locking it?
|
@superboyiii Require testing in x86 and x64 machines |
@shargon I added the |
OK |
@shargon any input would be nice. so we can get this going. |
| return db.Get(options, key); | ||
| } | ||
| public byte[] TryGet(byte[] key) => | ||
| _db.Get(key, _readOptions); |
There was a problem hiding this comment.
I think that if we use lock to write we also need it to read, or you could be reading while someone else is writing
There was a problem hiding this comment.
That's why you take/use a snapshot.
Description
This fixes the store's
dotnetmemory management forGC, a long with adding new feature options, easierExceptioncatching.Also exposes the store.
Change Log
ReadOptions,WriteOptionsandOptions.LevelDBHandle.csforGCand easy management.IEnumerable<KeyValuePair<byte[], byte[]>>toStore.cstoIterateover the whole store.namespacefromNeo.IO.Data.LevelDBtoNeo.IO.Storage.LevelDBMaxOpenFiles=4096,SnappyCompressionandFilterPolicy=10option toStore.thread-safetytobatchingsinceleveldbdoesn't supportmulti-threadin batching.Type of change
How Has This Been Tested?
Unit tests
Checklist: