<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
On Tue, Jul 6, 2021 at 7:55 PM Chris Johns <chrisj@rtems.org> wrote:<br>
><br>
> On 3/7/21 5:56 am, Alex White wrote:<br>
> > On Wed, Jun 30, 2021 at 11:40 PM <chrisj@rtems.org> wrote:<br>
> >><br>
> >> From: Chris Johns <chrisj@rtems.org><br>
> >><br>
> >> - The member variable `path_` cannot be a reference and initialised to<br>
> >> a const char* type input. To do so would require there is a temporary with<br>
> >> an unspecified life time.<br>
> >> ---<br>
> >> tester/covoar/AddressToLineMapper.h | 2 +-<br>
> >> tester/covoar/Target_aarch64.h | 2 +-<br>
> >> 2 files changed, 2 insertions(+), 2 deletions(-)<br>
> >><br>
> >> diff --git a/tester/covoar/AddressToLineMapper.h<br>
> > b/tester/covoar/AddressToLineMapper.h<br>
> >> index 88bf475..c78aef7 100644<br>
> >> --- a/tester/covoar/AddressToLineMapper.h<br>
> >> +++ b/tester/covoar/AddressToLineMapper.h<br>
> >> @@ -84,7 +84,7 @@ namespace Coverage {<br>
> >> * An iterator pointing to the location in the set that contains the<br>
> >> * source file path of the address.<br>
> >> */<br>
> >> - const std::string& path_;<br>
> >> + const std::string path_;<br>
> ><br>
> > Ryan alerted me about this issue a couple weeks back. This patch would fix the<br>
> > problem, but it would lead to a second copy of every file path string being<br>
> > stored in memory. This would also take away the usefulness of the set of file<br>
> > path strings maintained by the AddressLineRange class.<br>
> ><br>
> > Instead, I propose we change the empty SourceLine constructor to take a `const<br>
> > std::string&`. This would allow the addition of something like this to the top<br>
> > of AddressToLineMapper::getSource():<br>
> > const std::string unknown = "unknown";<br>
> > const SourceLine default_sourceline = SourceLine(unknown);<br>
> ><br>
> > That should eliminate the issue and preserve the memory conservation efforts of<br>
> > the original design.<br>
><br>
> Yes it would but for some reason, that I cannot put my finger on, it seems like<br>
> a breach of boundaries between classes.<br>
><br>
> How much data are we talking about? Are you able to see the memory foot print<br>
> with the strings being contained vs what you have now?<br>
<br>
Sorry for the late reply.<br>
<br>
What I have now yields a peak memory usage for covoar when run on all ARM tests of 219 MB.<br>
Changing `SourceLine::path_` to plain `std::string` results in an increase to 523 MB.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
So it is a significant increase.<br>
<br>
><br>
> If the figures show the strings need to be shared to avoid a memory blow out<br>
> would a std::shared_ptr<std::string> be something to look at where all<br>
> references are a shared pointer? A shared pointer means any changes do not flow<br>
> from one class to other.<br>
<br>
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?
<div>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.<br>
<br>
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?
<div>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:<br>
<br>
if (_path == nullptr) {<br>
return "unknown";<br>
else {<br>
return *_path;<br>
}<br>
<br>
Alex
<div><span class="sewnj24wcrxspuh"></span><span class="sewnj24wcrxspuh"></span><br>
><br>
> Chris<br>
> _______________________________________________<br>
> devel mailing list<br>
> devel@rtems.org<br>
> http://lists.rtems.org/mailman/listinfo/devel</div>
</div>
</div>
<br>
</div>
</body>
</html>