Switching from ArraySchema* to shared_ptr<ArraySchema> #2923
Conversation
stavrospapadopoulos
force-pushed
the
sp/schema_tdb_shared
branch
from
d1cadbd
to
870c3a0
Compare
There was a problem hiding this comment.
tdb_shared_ptr is a deprecated definition at this point and not for new code. std::shared_ptr is incorporated into our global namespace through common.h. You can simply drop the tdb_ prefix and everything should compile (assuming common.h has been added, which is most of the place by now).
I've not attempted an exhaustive review. Much of what I've said about Array also applies to FragmentInfo, for example.
| @@ -77,7 +77,7 @@ class ArraySchema { | |||
| * | |||
| * @param array_schema The array schema to copy. | |||
| */ | |||
| explicit ArraySchema(const ArraySchema* array_schema); | |||
| explicit ArraySchema(const tdb_shared_ptr<const ArraySchema>& array_schema); | |||
There was a problem hiding this comment.
From looking at this code previously, I know this constructor is only called once, in the C API function tiledb_array_get_schema() line 4772. As this PR changes allocation in that function, this constructor isn't called any more and can be removed.
There was a problem hiding this comment.
I still see this getting used btw.
There was a problem hiding this comment.
Which "this"? tiledb_array_get_schema() or ArraySchema(...)?
There was a problem hiding this comment.
Sorry, I am referring to the constructor. I attempted to remove it but it was still used in another place. I will punt for now.
stavrospapadopoulos
force-pushed
the
sp/schema_tdb_shared
branch
5 times, most recently
from
c011131
to
0076ce4
Compare
stavrospapadopoulos
force-pushed
the
sp/schema_tdb_shared
branch
4 times, most recently
from
375e8a1
to
50c8c6d
Compare
stavrospapadopoulos
force-pushed
the
sp/schema_tdb_shared
branch
from
50c8c6d
to
02f56ed
Compare
stavrospapadopoulos
changed the title
The motivation for this PR is to avoid fetching the latest array schema twice, as well as for code cleanup purposes.
We were fetching the latest array schema twice because of legacy code that was using raw
ArraySchema*pointers for the latest array schema everywhere, and smart pointers for the multiple array schemas used for versioning. This PR switches from using raw array schema pointers to shared pointers wherever possible. It also fetches the latest array schema once, by grabbing it from the versioned array schemas that are loaded anyway.TYPE: IMPROVEMENT
DESC: Switch to smart pointers and const references for
ArraySchema, and avoid fetching the latest array schema twice.