[Octave-patch-tracker] [patch #10147] interpreter: Avoid string construction

[Petter Tomner]

Follow-up Comment #5, patch #10147 (project octave):

[comment #4:]
> The first change seems OK, at least in functions that are called many many
times, but I'm not interested in changing all Octave code to use static const
std::string objects for every string literal.  The context matters.  For
example, there is no need for it in something like
> 
> 
> if (some_error_condition)
>   error ("...");
> 

Ye that would be silly. The __get_* function signatures could instead be
changed to 'const * char' to avoid the const static std::string, since they
are just used for a
'.c_str ()' in the end anyway.

> I don't think the other two changes are needed.  I'd rather avoid adding
things like "m_empty_string" member variables or const reference for ordinary
local variables.  Shouldn't std::move be used in those instances anyway, or
does something prevent that from happening?
> 

I think std::move is used (the move constructor) but I think there need to be
a malloc for the returned string anyways somewhere to have a copy. The
shenanigans 
with the "m_empty_string" is to allow a const reference for the "return m_rep
? m_rep->m_foo : m_empty_str". A copy of a empty string should not result
in any mallocs due to "short string optimizations", but a copy of m_foo might
trigger one depending on size.

I tried patch 2 in isolation and verified that it removes two mallocs per for
loop iteration. One because of changes in symscope and one because of changes
in fcn-info.

Removing const ref locals in fcn-info.cc with fcn-info.hh as in patch 2 did
not remove the malloc.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/patch/?10147>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/