924278 - use AString/ACString in IDL for shared string to avoid OOM / reduce memory usage

Status: NEW

(MailNews Core :: Backend, defect, P3)

Description

[Wayne Mery (:wsmwk)]

+++ This bug was initially created as a clone of Bug #837438 +++

bug 837438 comment 14
"We should use AString/ACString in IDL for shared string [to help avoid OOM].  Some cases can save memory."

Comment 1

[Wayne Mery (:wsmwk)]

(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #0)
> +++ This bug was initially created as a clone of Bug #837438 +++
> 
> bug 837438 comment 14
> "We should use AString/ACString in IDL for shared string [to help avoid
> OOM].  Some cases can save memory."
(more from the bug)
"We should use fallible allocator if we can handle OOM as possible.  But we need modify more codes, so tb24 is too short for it.  It needs more times."

might this be easy?

Comment 2

[Kent James (:rkent)]

I think that he is giving general principles of how the code should be written, and not making a specific recommendation for a change to fix a bug. I don't think it is a good idea to do a mass change of coding practice without a specific issue we are trying to solve.

Comment 3

[Wayne Mery (:wsmwk)]

(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #1)
> (In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #0)
> > +++ This bug was initially created as a clone of Bug #837438 +++
> > 
> > bug 837438 comment 14
> > "We should use AString/ACString in IDL for shared string [to help avoid
> > OOM].  Some cases can save memory."
> (more from the bug)
> "We should use fallible allocator if we can handle OOM as possible.  But we
> need modify more codes, so tb24 is too short for it.  It needs more times."
> 
> might this be easy?

m_kato, can you suggest for what types of Thunderbird actions or components this might be most helpful?

Comment 4

[Makoto Kato [:m_kato]]

(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #3)
> (In reply to Wayne Mery (:wsmwk, NI for questions) from comment #1)
> > (In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #0)
> > > +++ This bug was initially created as a clone of Bug #837438 +++
> > > 
> > > bug 837438 comment 14
> > > "We should use AString/ACString in IDL for shared string [to help avoid
> > > OOM].  Some cases can save memory."
> > (more from the bug)
> > "We should use fallible allocator if we can handle OOM as possible.  But we
> > need modify more codes, so tb24 is too short for it.  It needs more times."
> > 
> > might this be easy?
> 
> m_kato, can you suggest for what types of Thunderbird actions or components
> this might be most helpful?

All IDLs should use AString/AUTF8String/ACString instead of string/wstring.  string and wstring must always allocate memory for string, but nsAString(nsString) and nsACString(nsCString) don't.  Also if small string, we can use nsAutoString (allocated on stack) instead.

Even if we change this, addon that is written by JS keeps code compatibility.

Comment 5

[:aceman]

There are quite some hits in c-c:
https://dxr.mozilla.org/comm-central/search?q=wstring+file%3A*.idl+path%3Amailnews&redirect=false

It would mean to also change the type of argument of the respective functions implementing the methods/attributes, e.g. wstring in .idl means char16_t in .cpp, which would need to be changed to AString. Also the callers of the functions.

Comment 6

[Wayne Mery (:wsmwk)]

(In reply to Wayne Mery (:wsmwk) from comment #0)

+++ This bug was initially created as a clone of Bug #837438 +++
bug 837438 comment 14
"We should use AString/ACString in IDL for shared string [to help avoid OOM]. Some cases can save memory."

Are we likely in the next year to need to make changes in this area for architectural or other reasons? see also comment 5
TBH, I have no idea how much we'd save.

Comment 7

[Magnus Melin [:mkmelin]]

Nothing's forcing us, but it would of course be good for memory safety reasons (using built-in string classes).
Looks like we don't have too much of these now actually: https://searchfox.org/comm-central/search?q=string&case=true&regexp=true&path=mail*.idl

Comment 8

[Wayne Mery (:wsmwk)]

No one has quantified this, but also no one has suggested it is a major issue. So -> S4