Skip to content
This repository was archived by the owner on Mar 22, 2023. It is now read-only.

Add example for using pool as a class member#1032

Merged
igchor merged 1 commit intopmem:masterfrom
karczex:pool_example
Mar 25, 2021
Merged

Add example for using pool as a class member#1032
igchor merged 1 commit intopmem:masterfrom
karczex:pool_example

Conversation

@karczex
Copy link
Copy Markdown

@karczex karczex commented Feb 8, 2021

I added example for this, as it's not obvious at the first glance how
to handle pool as class member, and how to access it's data.


This change is Reviewable

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 8, 2021

Codecov Report

Merging #1032 (58a11eb) into master (eca8f5f) will decrease coverage by 0.19%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1032      +/-   ##
==========================================
- Coverage   94.25%   94.06%   -0.20%     
==========================================
  Files          48       48              
  Lines        4720     4750      +30     
==========================================
+ Hits         4449     4468      +19     
- Misses        271      282      +11     
Flag Coverage Δ
tests_clang_debug_cpp17 93.52% <ø> (-0.17%) ⬇️
tests_gcc_debug 92.01% <100.00%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
include/libpmemobj++/pool.hpp 88.17% <100.00%> (+2.27%) ⬆️
include/libpmemobj++/condition_variable.hpp 73.80% <0.00%> (-4.77%) ⬇️
include/libpmemobj++/transaction.hpp 79.83% <0.00%> (-2.74%) ⬇️
...libpmemobj++/detail/enumerable_thread_specific.hpp 98.91% <0.00%> (-1.09%) ⬇️
...ude/libpmemobj++/container/concurrent_hash_map.hpp 93.88% <0.00%> (-0.32%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eca8f5f...58a11eb. Read the comment docs.

Copy link
Copy Markdown
Contributor

@igchor igchor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 4 files at r1.
Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion (waiting on @karczex)


examples/pool/pool_as_class_member.cpp, line 26 at r1 (raw file):

	/* pool object */
	pmem::obj::pool<persistent_data> pop;
	/* Pointer to data. */

?

Copy link
Copy Markdown
Member

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 4 files at r1.
Reviewable status: 3 of 4 files reviewed, 6 unresolved discussions (waiting on @karczex)

a discussion (no related file):
in the commit message:

  1. Please drop "I added" ;) (and re-phrase I guess)
  2. it's data -> its data


examples/pool/pool_as_class_member.cpp, line 40 at r1 (raw file):

			std::cerr << "Cannot open pool: " << e.what()
				  << std::endl
				  << "Trying to create new one " << std::endl;

a new one


examples/pool/pool_as_class_member.cpp, line 44 at r1 (raw file):

							    PMEMOBJ_MIN_POOL);
		}
		/* Assign persistent poiner to root object. */
  1. pointer (misspell)
  2. to the root object

examples/pool/pool_as_class_member.cpp, line 64 at r1 (raw file):

	increment()
	{
		auto root_obj = pop.root();

maybe don't duplicate code - use set(x+1)


examples/pool/pool_as_class_member.cpp, line 72 at r1 (raw file):

	print()
	{
		auto root_obj = pop.root();

you don't need this line, just use it below

Copy link
Copy Markdown
Contributor

@KFilipek KFilipek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @karczex)


examples/pool/pool_as_class_member.cpp, line 28 at r1 (raw file):

	/* Pointer to data. */

	static constexpr const char *layout = "pool_layout";

const char* is not enough?


examples/pool/pool_as_class_member.cpp, line 41 at r1 (raw file):

			pop = pool<persistent_data>::create("poolfile", layout,
							    PMEMOBJ_MIN_POOL);

if pop is still nullptr then try to create instead of doing work under risk in catch clause outside of try block


examples/pool/pool_as_class_member.cpp, line 46 at r1 (raw file):

		/* Assign persistent poiner to root object. */
	}

Foo() = delete;

@karczex
Copy link
Copy Markdown
Author

karczex commented Mar 15, 2021


examples/pool/pool_as_class_member.cpp, line 72 at r1 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

you don't need this line, just use it below

Done.

@karczex
Copy link
Copy Markdown
Author

karczex commented Mar 15, 2021


examples/pool/pool_as_class_member.cpp, line 64 at r1 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

maybe don't duplicate code - use set(x+1)

Done.

@karczex
Copy link
Copy Markdown
Author

karczex commented Mar 15, 2021


examples/pool/pool_as_class_member.cpp, line 46 at r1 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

Foo() = delete;

Done.

@karczex
Copy link
Copy Markdown
Author

karczex commented Mar 15, 2021


examples/pool/pool_as_class_member.cpp, line 41 at r1 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…
			pop = pool<persistent_data>::create("poolfile", layout,
							    PMEMOBJ_MIN_POOL);

if pop is still nullptr then try to create instead of doing work under risk in catch clause outside of try block

Done.

@karczex
Copy link
Copy Markdown
Author

karczex commented Mar 15, 2021


examples/pool/pool_as_class_member.cpp, line 40 at r1 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

a new one

Done.

@karczex
Copy link
Copy Markdown
Author

karczex commented Mar 15, 2021


examples/pool/pool_as_class_member.cpp, line 28 at r1 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

const char* is not enough?

Done.

@karczex
Copy link
Copy Markdown
Author

karczex commented Mar 15, 2021


examples/pool/pool_as_class_member.cpp, line 26 at r1 (raw file):

Previously, igchor (Igor Chorążewicz) wrote…

?

Done.

@karczex karczex force-pushed the pool_example branch 2 times, most recently from a376a3b to 4f86e17 Compare March 15, 2021 17:32
@karczex
Copy link
Copy Markdown
Author

karczex commented Mar 15, 2021

a discussion (no related file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

in the commit message:

  1. Please drop "I added" ;) (and re-phrase I guess)
  2. it's data -> its data

Done.


Copy link
Copy Markdown
Member

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 3 files at r2.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @igchor, @karczex, and @KFilipek)


include/libpmemobj++/pool.hpp, line 47 at r2 (raw file):

 * @snippet pool/pool.cpp pool_base_example
 *
 * The exmple of using pool with RAII idom:

idiom

Copy link
Copy Markdown
Author

@karczex karczex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 3 of 4 files reviewed, 5 unresolved discussions (waiting on @igchor, @KFilipek, and @lukaszstolarczuk)


examples/pool/pool_as_class_member.cpp, line 44 at r1 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…
  1. pointer (misspell)
  2. to the root object

Done.


include/libpmemobj++/pool.hpp, line 47 at r2 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

idiom

Done.

Copy link
Copy Markdown
Member

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @igchor and @KFilipek)

Copy link
Copy Markdown
Contributor

@KFilipek KFilipek left a 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, 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @igchor)

Copy link
Copy Markdown
Contributor

@KFilipek KFilipek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @igchor)

Copy link
Copy Markdown
Contributor

@igchor igchor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @karczex)

@igchor igchor merged commit 2bc7289 into pmem:master Mar 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants