Skip to content
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

added load&dump interface #22

Merged
merged 6 commits into from Aug 18, 2019
Merged

added load&dump interface #22

merged 6 commits into from Aug 18, 2019

Conversation

gtarcoder
Copy link
Contributor

No description provided.

@gtarcoder
Copy link
Contributor Author

gtarcoder commented Aug 14, 2019

@greg7mdp Hi Gregory, I want to add load and dump interface for set&map instead of using cereal.

I do this because I need to dump and load the map/set very quickly for arithmetic types, e.g. map<uint32_t, uint64_t>, fortunately all necessary data of these types are stored in ctrl_ and slots_ area so we can dump&load ctrl_ and slots_ directly.

Could you help to review that? Any comment would be appreciated.

@gtarcoder gtarcoder force-pushed the master branch 3 times, most recently from 6ba4d3a to 48c477b Compare August 15, 2019 11:48
@greg7mdp
Copy link
Owner

Hi @gtarcoder,

Thanks for the change. The tests fail line 3298:

/Users/travis/build/greg7mdp/parallel-hashmap/parallel_hashmap/phmap.h:3298:34: error: use of undeclared identifier 'dump_path'
if (!inner.set_.dump(dump_path)) {

@gtarcoder
Copy link
Contributor Author

Hi @gtarcoder,

Thanks for the change. The tests fail line 3298:

/Users/travis/build/greg7mdp/parallel-hashmap/parallel_hashmap/phmap.h:3298:34: error: use of undeclared identifier 'dump_path'
if (!inner.set_.dump(dump_path)) {

Hi @greg7mdp , have fixed errors now. And have made more changes to this PR, any comments ?

@greg7mdp greg7mdp merged commit ba121d0 into greg7mdp:master Aug 18, 2019
@greg7mdp
Copy link
Owner

The IsStringOrArithmeticType check is not right, If you used long strings in your test you would see that the dump does not work because the string has an internal pointer. In any case I think you should use is_trivially_copyable instead because this is exactly what we need for the dump.

Somehow I tried updating your change and I messed up my local github repository... trying to fix it - I have a support ticket with github.

@greg7mdp
Copy link
Owner

Hi @gtarcoder , I merged your pull request by mistake, and then I undid it. Sorry about that!

@greg7mdp
Copy link
Owner

greg7mdp commented Aug 19, 2019

@gtarcoder , please submit a new pull request.

@gtarcoder
Copy link
Contributor Author

@gtarcoder , please submit a new pull request.

Hi @greg7mdp , sorry for troubles, I will create a new PR.

@ktprime ktprime mentioned this pull request Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants