Open Bug 76669 Opened 24 years ago Updated 2 years ago

performance enhancement to the style system in regard with XLinks

Categories

(Core :: XML, enhancement, P3)

enhancement

Tracking

()

Future

People

(Reporter: sbanu, Unassigned)

Details

(Keywords: perf)

Attachments

(1 file)

From Bugzilla Helper: User-Agent: Mozilla/4.0 (compatible; MSIE 5.01; Windows NT 5.0) BuildID: Mozilla 0.8.1 There is about the state of a link. Finding out a state can be a little bit improved in order to have better performance. This patch will help other developers to have there own type of links, not supported by Mozilla.
Attached patch Patch for BUG 76669 (deleted) — Splinter Review
Thanks Sorin, I'll try to review (or get someone in the style group) to review this by the end of this week, or early next week.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: perf
I don't think this approach would work. You should not cache the link state in the element, because the element should not care about its state - and in fact it cannot even know its state. There can be scripts that change the href (and document base href or XML Base for some ancestor of the element once we start using them in the style system - right now they are too slow), in effect changing the URL of the element so link state might need to be changed. Additionally once we get extended XLinks this will not fly very well. However, I do believe we can speed things up. Either by adding a new method to nsIContent (my preference) or nsIXMLContent we should be able to gain some speed. Here is the method signature: NS_IMETHOD GetIsLinkStart(PRBool *aIsLinkStart) = 0; XML elements already have a member variable mIsLink. Most HTML elements would simply return false. 'A' and other's could return true. The reason I prefer nsIContent is because we are trying to get rid of nsIXMLContent. With that method we could simplify code and get a performance boost. Most elements in a document would not be links, so only this one function call would be required for the most part. If it was a link, we would still do a lot of work, though. However, I see that HTML is caching the link state in the element. I don't know if it represents the reality if a script changes the href value of the element, or the document base href. I would guess it does not work. This could be helped somewhat with 'dirty bits' in the element and document, but it would slow down all mutation tasks a little. I Cc:d a bunch of people who were actively involved in the previous round of :visited performance work.
You know that all I wanted was something that can help us use the style part (in this case showing links in the way they should be shown) without modifing the mozilla code. For this is needed that function GetIsLink(...) or GetIsLinkStart(...) and of course used in nsStyleUtil.cpp in order to find about an element if it is link or not. But the rest: asking for the href attribute and its value are specific only to SimpleXLink elements, so it should be another function in nsStyleUtil, something like IsOtherLinkStart(...), and here comes the problem of state. You cannot find out a state of a link only if you interogate the element (nsIContent or whateverContent) (in the simple XLink case, the "href" attribute). Another thing that tells me that the best place to keep the state of the element is the element itself: what if the href keeps a XPointer URI... something like http://www.mozilla.org#/1/2/3 and this is the same target with http://www.mozilla.org#an_id , then of course the GetLinkState(...) from the linkHandler will not gives us the right link state if the link was never visited from a link that has the XPointer_kind_of href... and there could be other ways of representing a target in a given base URI. Is not your fault, Heikki, that Mozilla is implemented in the way it is... anyway, even if the logic way is not to keep the state in the element, the better speed way i might say it is. But, you are right about my state implementation in element, it was just the idea and not finished... so in the way it is, yes, PRBool mIsLink is enough and better. A way of solving the problem is not considering a link visited only when the target was riched (eg links to #id1 are marked visited if that #id1 was a target of a previous scroll(click)). A link is considered visited if its target is in a visited document. So, we should look at the baseURI of the document (http://www.mozilla.org) not at the whole URI (http://www.mozilla.org#id1)... this is the way iexplorer does and the way I consider is normal... and of course helps us get rid a lot of problems. If then is made a change of the target ("href" attribute value in the simple XLink case) by jscript or whatever, then we could simply watch for these changes in SetAttribute(...) function of the element: ask if it is link and, of course, only if it is ask if the attribute is a href or other link_target_type attribute, then check out the link state of this new href value and set it in the element... in this way we are sure the check of the state is made only on when modified. "type" attribute changed from "simple" to something else gives us a non-link element and other combinations/problems like this. A better way of doing this might be having a nsIContentLink deriving from nsIContent that has these attribute functions or whatever_we_need_to_detect_a_link functions specific, this way speeding up the things, but growing up in memory :(. Anyway, Heikki, i will be pleased if only that GetIsLinkStart is added and make a element looks like a link if it is link, for the moment (0.9)... using in nsStyleUtil something like IsOtherLinkStart(...) after IsXLinkStart(...). Maybe you will accept this status in a element thought, even that you have no absolute need of it (except that discutable speed improvement), because in this way someone can set from its component the status of its implemented type of links (like DocZilla case)... but I agree the way I did that patch was not quite comprehensive. I am working on complete XLink support right now, as I told you, and maybe, if I have time I will try to find a solution for this visited problem too. A... and think at that "all the targets in a visited document should be considered visited" issue... shouldnt it be like this? It is a hell to take care of all the targets in a document, as you said, just thinking at all the modifications that can be done at run time in a DOM of a document and all the ways a target can be represented. Phew! hope I wont have to write that much next time... cause i hate reading too.
Priority: -- → P3
Target Milestone: --- → mozilla1.0
Target Milestone: mozilla1.0 → mozilla0.9.9
Target Milestone: mozilla0.9.9 → mozilla1.0
Target Milestone: mozilla1.0 → mozilla1.0.1
Target Milestone: mozilla1.0.1 → mozilla1.2alpha
Target Milestone: mozilla1.2alpha → Future
QA Contact: petersen → rakeshmishra
QA Contact: rakeshmishra → ashishbhatt
Has there been any work on this bug?
Anne in comment #5: > Has there been any work on this bug? nothing will be coming from the reporter - email>Sorin bounces
QA Contact: ashshbhatt → xml
Assignee: hjtoi-bugzilla → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: