461951 - Selecting a message for display, can nsMsgMailNewsURL implement nsIMutable and avoid being cloned for docshell loads? And reduce memory fragmentation

Status: RESOLVED WORKSFORME

(MailNews Core :: Backend, defect)

Description

[Mark Banner (:standard8)]

I just noticed that when we select a different message to display, we are creating new instances of nsMsgMailNewsURL approximately 9 times for a simple message load, for a more complex message I've seen 18-20 instances being created.

From looking at the debugger, most of these are clones:

7 out of 9 were clones for the basic case
14 out of 19 for a complex case

These seem to be caused by NS_EnsureSafeToReturn. This is a security hook that ensures at various times when getting urls to check for security purposes (or where they are exposed) that the returned url can either not be modified, or not modify the original url. This is determined by the nsIMutable interface.

Given that we should be cutting down allocations where possible, this is one place that could buy us some performance (and reduce memory fragmentation).

I think this would be an improvement (even with the addition of the nsIMutable implementation) but compared to message content loading allocations etc, it may not actually gain us that much.

Hence I'm filing it as a possibility for further investigation, and setting wanted tb 3. If someone wants to take a closer look at this please do.

Comment 1

[Wayne Mery (:wsmwk)]

Is this even still an issue?

Comment 2

[Wayne Mery (:wsmwk)]

Nomis, can you check in debugger?  (So much has changed in backend in 8 years)

Comment 3

[Nomis101]

(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #2)
> Nomis, can you check in debugger?  (So much has changed in backend in 8
> years)

I will check on the weekend.

Comment 4

[Wayne Mery (:wsmwk)]

(In reply to Nomis101 from comment #3)
> (In reply to Wayne Mery (:wsmwk, NI for questions) from comment #2)
> > Nomis, can you check in debugger?  (So much has changed in backend in 8
> > years)
> 
> I will check on the weekend.

any luck?

Comment 5

[Nomis101]

(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #4)
> (In reply to Nomis101 from comment #3)
> > (In reply to Wayne Mery (:wsmwk, NI for questions) from comment #2)
> > > Nomis, can you check in debugger?  (So much has changed in backend in 8
> > > years)
> > 
> > I will check on the weekend.
> 
> any luck?
Sorry, I've forgotten about this, hope I will find time on Sunday...

Comment 6

[Nomis101]

(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #4)
> (In reply to Nomis101 from comment #3)
> > (In reply to Wayne Mery (:wsmwk, NI for questions) from comment #2)
> > > Nomis, can you check in debugger?  (So much has changed in backend in 8
> > > years)
> > 
> > I will check on the weekend.
> 
> any luck?

I've tested today (TB 52, macOS 10.12) and I was seeing in my case 2 instances of nsMsgMailNewsURL per message load.

Comment 7

[Wayne Mery (:wsmwk)]

(In reply to Nomis101 from comment #6)

I've tested today (TB 52, macOS 10.12) and I was seeing in my case 2
instances of nsMsgMailNewsURL per message load.

thanks Nomis101 !

Kaie, given the above in context of comment 0, how do you feel about closing this WFM?

Comment 8

[Kai Engert (:KaiE:)]

sounds good