-
Notifications
You must be signed in to change notification settings - Fork 33
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
Move dictEntry to PM #135
base: 6.0-memkind
Are you sure you want to change the base?
Move dictEntry to PM #135
Conversation
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.
Reviewable status: 0 of 7 files reviewed, 1 unresolved discussion (waiting on @karpiop)
src/dict.h, line 156 at r1 (raw file):
dictEntry *dictAddRawPM(dict *d, void *key, dictEntry **existing);
not used anywhere remove
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.
Reviewed 7 of 7 files at r1.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @karpiop)
src/db.c, line 180 at r1 (raw file):
* The program is aborted if the key already exists. */ void dbAdd(redisDb *db, robj *key, robj *val) { sds copy;
= NULL;
src/db.c, line 185 at r1 (raw file):
else copy = sdsdup(key->ptr); int retval;
int retval = 0;
Please don't leave uninitialized values
src/dict.h, line 155 at r1 (raw file):
int dictAdd(dict *d, void *key, void *val); int dictAddPM(dict *d, void *key, void *val); dictEntry *dictAddRaw(dict *d, void *key, dictEntry **existing, int dictionary_entries_on_pmem);
camelCase, same as dictCreate
src/dict.h, line 157 at r1 (raw file):
dictEntry *dictAddRaw(dict *d, void *key, dictEntry **existing, int dictionary_entries_on_pmem); dictEntry *dictAddRawPM(dict *d, void *key, dictEntry **existing); dictEntry *dictAddOrFind(dict *d, void *key, int dictionary_entries_on_pmem);
camelCase, same as dictCreate
src/dict.c, line 292 at r1 (raw file):
* If key was added, the hash entry is returned to be manipulated by the caller. */ dictEntry *dictAddRaw(dict *d, void *key, dictEntry **existing, int dictionary_entries_on_pmem)
camelCase, same as dictCreate
src/dict.c, line 292 at r1 (raw file):
* If key was added, the hash entry is returned to be manipulated by the caller. */ dictEntry *dictAddRaw(dict *d, void *key, dictEntry **existing, int dictionary_entries_on_pmem)
Maybe const int* to avoid copy creation
src/dict.c, line 381 at r1 (raw file):
* * See dictAddRaw() for more information. */ dictEntry *dictAddOrFind(dict *d, void *key, int dictionary_entries_on_pmem) {
camelCase, same as dictCreate
src/dict.c, line 383 at r1 (raw file):
dictEntry *dictAddOrFind(dict *d, void *key, int dictionary_entries_on_pmem) { dictEntry *entry, *existing; entry = dictAddRaw(d,key,&existing,dictionary_entries_on_pmem);
camelCase, same as dictCreate
src/expire.c, line 442 at r1 (raw file):
if (db->id > 63) return; dictEntry *de = dictAddOrFind(slaveKeysWithExpire,key->ptr,server.dictionary_entries_on_pmem);
variables above have also camelcase naming
7a35632
to
a4271ce
Compare
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.
Reviewable status: 4 of 7 files reviewed, 10 unresolved discussions (waiting on @karpiop, @KFilipek, and @michalbiesek)
src/db.c, line 180 at r1 (raw file):
Previously, KFilipek (Krzysztof Filipek) wrote…
= NULL;
Done.
src/db.c, line 185 at r1 (raw file):
Previously, KFilipek (Krzysztof Filipek) wrote…
int retval = 0;
Please don't leave uninitialized values
Done.
src/dict.h, line 155 at r1 (raw file):
Previously, KFilipek (Krzysztof Filipek) wrote…
camelCase, same as dictCreate
Done.
src/dict.h, line 156 at r1 (raw file):
Previously, michalbiesek wrote…
dictEntry *dictAddRawPM(dict *d, void *key, dictEntry **existing);
not used anywhere remove
Done.
src/dict.h, line 157 at r1 (raw file):
Previously, KFilipek (Krzysztof Filipek) wrote…
camelCase, same as dictCreate
Done.
src/dict.c, line 292 at r1 (raw file):
Previously, KFilipek (Krzysztof Filipek) wrote…
camelCase, same as dictCreate
Done.
src/dict.c, line 381 at r1 (raw file):
Previously, KFilipek (Krzysztof Filipek) wrote…
camelCase, same as dictCreate
Done.
src/dict.c, line 383 at r1 (raw file):
Previously, KFilipek (Krzysztof Filipek) wrote…
camelCase, same as dictCreate
Done.
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.
Reviewable status: 4 of 7 files reviewed, 12 unresolved discussions (waiting on @karpiop and @KFilipek)
src/dict.c, line 334 at r2 (raw file):
Quoted 10 lines of code…
int dictAddPM(dict *d, void *key, void *val) { (void)(d); (void)(key); (void)(val); printf("ERROR: dictAddPM is supported only by memkind\n"); exit(1); /* unreachable */ return NULL; }
not needed now - remove
this applies also to USE_MEMKIND
src/dict.c, line 345 at r2 (raw file):
} #endif
As we spoke offline extend commit message to be more informative - maybe what scenario are covered
src/t_zset.c, line 2335 at r2 (raw file):
0
Maybe some define(s) for '0' and '1' value ? - I am not sure what header would be good to place it, but I guess you could figure it out.
a52e170
to
25f80f9
Compare
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.
Reviewable status: 1 of 7 files reviewed, 10 unresolved discussions (waiting on @karpiop and @KFilipek)
src/db.c, line 185 at r1 (raw file):
Previously, karpiop (Paweł Karpiński) wrote…
Done.
IMHO this could be completely avoided using ternary operator:
int retval = (server.dictionary_entries_on_pmem) ?
dictAddPM(db->dict, copy, val) : dictAdd(db->dict, copy, val);
sds copy = (server.keys_on_pmem) ?
sdsdupPM(key->ptr) : sdsdup(key->ptr);
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.
Reviewed 1 of 3 files at r2, 5 of 5 files at r4.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @karpiop)
dictEntry instances will be allocated to PM. Involves dictionary entries created using DB API (db.c)
25f80f9
to
81c353e
Compare
[4.0.11-devel] Add compile and linking security flags
dictEntry instances will be allocated to PM. Involves only main dictionary entries.
This change is