Conversation
lexprfuncall
force-pushed
the
malloc-conf-cleanup
branch
from
6ddd694 to
98c1f2b
Compare
lexprfuncall
force-pushed
the
malloc-conf-cleanup
branch
2 times, most recently
from
0011bbe to
9ae3e7d
Compare
| } | ||
|
|
||
| void | ||
| malloc_conf_init(sc_data_t *sc_data, unsigned bin_shard_sizes[SC_NBINS], |
There was a problem hiding this comment.
As the only public interface of conf.h, maybe we can add a unit test for it as well? While most underneath logic is already covered by the current unit tests, the only one left would be to check if the two ordered calls to malloc_conf_init_helper is working properly, i.e., some options should be checked first before others.
There was a problem hiding this comment.
Sure, test coverage has now been added.
|
Two more general questions: 1. regarding the naming, would it be better if we call the new files |
lexprfuncall
force-pushed
the
malloc-conf-cleanup
branch
from
9ae3e7d to
33f4441
Compare
test/unit/conf_init_2.c
Outdated
| #include "test/jemalloc_test.h" | ||
|
|
||
| const char *malloc_conf = "dirty_decay_ms:1234,muzzy_decay_ms:2000"; | ||
| const char *malloc_conf_2_conf_harder = "dirty_decay_ms:5678"; |
There was a problem hiding this comment.
Other tests, especially conf_init_confirm, test are good. This test seems to serve the same purpose as malloc_conf_2.c but this new one is more extended. Keeping only one of them is good?
There was a problem hiding this comment.
The difference is that the malloc_conf test uses the environment variable and my tests do not. But it was my intention to carry over that functionality, I just forgot about that detail. Let me fix that.
There was a problem hiding this comment.
I opted to delete the new test and expand the coverage of the existing malloc_conf_2 test that test the environment variable.
lexprfuncall
force-pushed
the
malloc-conf-cleanup
branch
from
33f4441 to
3e834c4
Compare
I'd hope that HPA opts goes away through a rationalization of our implementation. That said, along the lines of what you suggest, I think there is a need for a bigger rationalization of the configuration support. Let's make it a topic for discussion in the near future. |
2 participants
This change moves the configuration code out of the initialization
code and into its own source code module that can be tested in
isolation. In addition to extracting the code, tests for the option
parsing have been added.