On Tue, Jul 6, 2021 at 7:55 PM Chris Johns <chr...@rtems.org> wrote: > > On 3/7/21 5:56 am, Alex White wrote: > > On Wed, Jun 30, 2021 at 11:40 PM <chr...@rtems.org> wrote: > >> > >> From: Chris Johns <chr...@rtems.org> > >> > >> - The member variable `path_` cannot be a reference and initialised to > >> a const char* type input. To do so would require there is a temporary > >> with > >> an unspecified life time. > >> --- > >> tester/covoar/AddressToLineMapper.h | 2 +- > >> tester/covoar/Target_aarch64.h | 2 +- > >> 2 files changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/tester/covoar/AddressToLineMapper.h > > b/tester/covoar/AddressToLineMapper.h > >> index 88bf475..c78aef7 100644 > >> --- a/tester/covoar/AddressToLineMapper.h > >> +++ b/tester/covoar/AddressToLineMapper.h > >> @@ -84,7 +84,7 @@ namespace Coverage { > >> * An iterator pointing to the location in the set that contains the > >> * source file path of the address. > >> */ > >> - const std::string& path_; > >> + const std::string path_; > > > > Ryan alerted me about this issue a couple weeks back. This patch would fix > > the > > problem, but it would lead to a second copy of every file path string being > > stored in memory. This would also take away the usefulness of the set of > > file > > path strings maintained by the AddressLineRange class. > > > > Instead, I propose we change the empty SourceLine constructor to take a > > `const > > std::string&`. This would allow the addition of something like this to the > > top > > of AddressToLineMapper::getSource(): > > const std::string unknown = "unknown"; > > const SourceLine default_sourceline = SourceLine(unknown); > > > > That should eliminate the issue and preserve the memory conservation > > efforts of > > the original design. > > Yes it would but for some reason, that I cannot put my finger on, it seems > like > a breach of boundaries between classes. > > How much data are we talking about? Are you able to see the memory foot print > with the strings being contained vs what you have now?
Sorry for the late reply. What I have now yields a peak memory usage for covoar when run on all ARM tests of 219 MB. Changing `SourceLine::path_` to plain `std::string` results in an increase to 523 MB. So it is a significant increase. > > If the figures show the strings need to be shared to avoid a memory blow out > would a std::shared_ptr<std::string> be something to look at where all > references are a shared pointer? A shared pointer means any changes do not > flow > from one class to other. I don't think that `std::shared_ptr` would help here. Wouldn't that handle the case of an unknown path the same way as a raw pointer solution? Wouldn't we still have to check the return value of `SourceLine::path()` to make sure that it is not null? The only other way I see to make it work would be to store some "unknown" string object and hand pointers to it to all `SourceLine` objects with unknown paths. But that seems mostly equivalent to what I propose in my original reply to this patch. Now that I think about it, maybe making `SourceLine::_path` into an `std::string*` and doing a `nullptr` check in the `SourceLine::path()` would be most elegant? That way we don't have to do any `nullptr` checks outside of the `SourceLine` implementation. We could just have `SourceLine::path()` return a plain old `std::string` and do something like this: if (_path == nullptr) { return "unknown"; else { return *_path; } Alex > > Chris > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel