3367 - Iterating over all arcs out in RDFTreeBuilderImpl::AddTreeRow

Status: VERIFIED FIXED

(Core Graveyard :: RDF, defect, P2)

Description

[scottputterman]

In RDFTreeBuilderImpl::AddTreeRow all of the arcs leading out of a node are
iterated over.  Especially in the case of building up children, this can require
a lot of extra time for information that isn't needed at the moment.

Comment 1

[scottputterman]

This is turning out to be a problem with mail folders.  Since we say that we
have the MessageChild property we are being asked for the messages for each
folder which can cause a problem for people with lots of folders and lots of
messages in those folders.

Is there an easy way to make it so that the containment property doesn't need to
take part in this?

Comment 2

[Chris Waterson]

Set target milestone to M4.

Comment 3

[Chris Waterson]

Set target milestone to M5.

Comment 4

[Chris Waterson]

Slipping to M5.

Comment 5

[scottputterman]

What's the likelihood of this being in M5?

Comment 6

[Chris Waterson]

Slipping to M6. This needs to be fixed immediately after M5 ships. Talked with
putterman today, and we agreed there are three things that need to happen:

1. Change the semantics of nsIRDFDataSource::ArcLabelsOut() to mean "possible"
arc labels out. That is, a datasource is allow to say it has an arc out, and a
subsequent GetTarget[s] may return no value. FWIW, ditto for ArcLabelsIn().

2. The test that determines whether an element is a container needs to use
GetTarget(). This is an implication of (1): we currently use ArcLabelsOut() and
look for the containment property.

3. (This is the tough one.) We need to alter nsRDFElement and the builder code
so that attribute values aren't eagerly computed and cached in the element.
Instead, we need to have attribute requests fall through to the graph. This may
have some evil implications with respect to the inline-style stuff that Hyatt
added a couple weeks ago.

Comment 7

[Robert John Churchill]

One other possible option that came up while Chris an dI were talking about this
is that, just as there is currently a "container='...'" attribute, we could add
something like a "ignore='...'" attribute which could be used in the mail folder
pane to ignore things like "MessageChild".  (Its skanky; I just thought I'd add
it here for completeness.)

Comment 8

[Chris Waterson]

Okay, I've given up on the "right way" of fixing it. Stuff is too tangled now.
I'm going to implement Robert's solution, that is, you'll be able to specify an
"rdf:ignore" property in the XUL that will tell the content model builder to
_not_ try to traverse the property. So that supercedes (3) in the list above.

Comment 9

[Chris Waterson]

Checked in the necessary changes to make this happen. Robert, in the process, I
had to #ifdef XUL_TEMPLATE the call to CreateWidgetItem().

Comment 10

[rubydoo123]

since this is codelevel goodies, Scott can you please verifiy this one since you
were the reporter.

Comment 11

[scottputterman]

From what I can see, it seems to be working.  I'll file a new bug if anything
comes up.  But right now, if rdf:ignore is used, then GetTarget is no longer
being called.

This doesn't solve the problem of potentially wanting lazy arc discovery, as
you've mentioned, but unless something else comes up, this will be fine.
Thanks for taking care of this.