Dr said:
Sure , I don't mind at all discussing the challenges,
as while I know a great deal ,there is always more
to discover.
The basic problem is that the menu system is designed to
work without knowing in advance what it's working with,
and to be reasonably tolerant of third party content while
allowing same to be possibly less than valid.
Since different browsers rendering the same random content will
tend to produce variably sized results, the menu system renders
and measures at page load. Needless to say this does present
some overhead, which at load time just sqeaks in to the realm
of acceptable delay.
To do so post load tends to make for a rather obvious thunk.
Of course every real application of browser scripting has a specific
context and so does not need to be truly general. So, given that no
script needs to be truly general, when the act of trying to be general
starts to get in the way of viable performance that might flag the need
for a design re-think.
But it is not necessarily the attempt to be general that results in your
code running like cold treacle, more the strategy taken in achieving
that generality. You have chosen a style of "meta" scripting, where you
are continually constructing strings of javascript source code (mostly
dot-notation property accessors) and then using - eval - to execute
them. Eval use introduces considerable overheads, varying in its impact
by as much as a factor of 20, depending on the execution environment.
The same applies to string concatenation, which is an area where some
common implementations perform particularly badly.
This "meta" style is entirely avoidable, and a more normal strategy of
coping whit attempting to interact with multiple documents in a frameset
through switching object references would certainly perform considerably
better (and be easier to understand), though only a small proportion of
applications actually use a frameset.
However, the entire design stile is not entirely responsible for the
sluggish performance. Much of the actual code is significantly
sup-optimal. Taking a random function as an example:-
| function DCcssApply(pL,szClass,oD)
| {
| oL=DCok(pL)
| if(typeof oD=='undefined')oD=DCgetParentWindow(pL).document;
| var oCSS =DCgetCSSclass(oD,szClass)
| szSkipTypes = ":function:undefined:"
| szSkipped = ""
| var oV;
| for(i in oCSS)
| {
| if(String("0123456789").indexOf(i.substring(0,1))>-1)continue;
| oV=eval("oCSS."+i);
| oT=typeof oV;//eval("oCSS."+i);
| if(szSkipTypes.indexOf(oT) > 0)continue;
| if(oDC.szCopy.indexOf(":"+i+":") == -1)
| {
| szSkipped+=i+"\n"; continue;
| }
| if(oV.indexOf("url(")>-1)
| {
| oV=oV.split("'").join("").split("(").join("('")
| .split(")").join("')")
| }
| szE='oL.style.'+i+'=oV'
|
| try { eval(szE)
| }catch(e)
| { DCdebug("Sucks "+szE) }
| }// End For{}
| }
The - oL - variable appears to have some role as a global variable but -
szSkipTypes - and - szSkipped - do not appear to be referenced by any
other code, and the - i - used in the for loop looks like it really is a
bad choice for a global variable. The scope chain resolution of
Identifiers will take longer the further down the scope chain an object
is found with the corresponding property name. The Variable/Activation
object, with property names corresponding with the local variables, is
at the top of the scope chain, while the global object, which property
names corresponding with the global variables, is always the last item
on the scope chain. So, apart form the questionable wisdom of allowing
variables that appear to only be useful within a single function leak
out into the global scope, using local variables in their place will
result in better performance.
The line:-
if(typeof oD=='undefined')oD=DCgetParentWindow(pL).document;
would execute faster as:-
if(oD)oD=DCgetParentWindow(pL).document;
- because it avoids the typeof and comparisons operations, and it is
safer because some non-undefined values would be just as erroneous as an
undefined argument.
if(String("0123456789").indexOf(i.substring(0,1))>-1)continue;
- includes String("0123456789"), where the string "0123456789" is
unambiguously a string primitive, making the call to the - String -
function to type-convert its argument into a string primitive is
redundant, and so pure overhead. The whole statement would probably be
faster if replaced with a regular expression test to see whether the
fist character in - i - is a decimal digit.
oV=eval("oCSS."+i);
- is just plain silly as - oCSS - is a local variable that refers to a
CSS rule's - style - property and - i - is one of its property names (as
acquired with for-in). The - eval - call is unnecessary to resolve the
value referred to by the property name - i -, instead a bracket notation
property accessor should be used, i.e:- oV = oCSS[ i ];
oT=typeof oV;//eval("oCSS."+i);
if(szSkipTypes.indexOf(oT) > 0)continue;
- are a couple of odd statements. The global string - szSkipTypes -
could just as easily be a globally declared object such as:- szSkipTypes
= {'fucntion':true,'undefined':true); -, allowing:-
if(szSkipTypes[(typeof oV)]){
continue;
}
But given the - oV - is a property of a - style - object, and may be a
method of that object, the key 'object' should probably be added to that
set as some hosts report 'object' as the result of a - typeof - test
applied to a method of a host object. It is also questionable whether
any type other than 'string' should be allowed past this test as the
value will be subject to the - indexOf - method, with which boolean and
numeric values would error-out, though excluding numbers would suffer
from excluding zIndex.
Note, however that:- szSkipTypes = ":function:undefined:" - means that
the - indexOf - method will have to examine and skip the colon character
at the start of the string, and the second colon if its argument happens
to be "undefined". There is no reason to have any of the colons in that
string.
The following:-
if(oDC.szCopy.indexOf(":"+i+":") == -1)
{
szSkipped+=i+"\n"; continue;
}
- would also benefit form indexing an object reference, avoiding the
need for the two concatenation operations and the comparison, and almost
certainly resolving faster. The following - szSkipped+=i+"\n"; - seems a
bit pointless as there do not appear to be any other references to the
global szSkipped variable, or much point in recording the enumerated
methods and properties with undefined values every time the function is
executed.
if(oV.indexOf("url(")>-1)
- will, as I have said, error-out in the event that - oV - is numeric,
as it may be for the -zIndex - property of a style object. There is also
an implicit assumption that enumerated properties of a style object will
never include object references, null values and boolean values, making
this code risky to expose to any browsers outside of a limited known set
(so not even attempting to be cross-browser).
oV=oV.split("'").join("").split("(").join("('")
.split(")").join("')")
- assumes - oV - is a string, creates an Array, turns it into a string,
creates another Array, turns that into a string, creates a third Array,
and turns that into a string. The whole process could probably be
replaced with a single String.prototype.replace call, and be
considerably faster in doing so.
In:-
szE='oL.style.'+i+'=oV'
try { eval(szE)
- oL - has not been verified as an object reference with a 'style'
property that refers to an object, but assuming it is , the eval call
can again be replaced with a much more efficient bracket notation
property accessor:-
try{
o.style[ i ] = oV;
It is also a little perverse to find:-
oDC.NS =(document.layers )?true:false
- and code that then branches based on the value of - oDC.NS -,
suggesting an attempt (if flawed) to accommodate Netscape 4, when
Netscape 4 will generate a syntax-error when it encounters the try-catch
in the source code, and so never even attempt to execute the script.
While all of these small changes are mostly only going to make a tiny
contribution to the performance of the script, the fact that a small
optimisation is available for nearly every line of code used would add
up. But in the end getting rid of the "meta" scripting, and all of the
consequent - eval - uses, would make a bigger contribution to solving
the performance problem. Abandoning the generalised monolithic library
concept in favour of targeting code re-use at a lower, more single task
specific, level would allow efficient tailoring to the context of
application and so largely avoid the overheads involved in using general
code in a specific context.
My current task is to finish the part that allows the
javascript code to read stylesheets in Opera,
Then which, Konqueror, Safari, IceBrowser, NetFront?
which as you are probably aware is
less than straight forward. (but doable).
I don't understand why you are doing this at all as it appears to
pre-suppose that all styles are applied with classes (rather than with
ID and elements selectors or context related and mixed selectors), but
it is unclear why an assignment to an element's - className - property
would not equally satisfy the requirement and considerably less effort
and trouble.
Again keep in mind that the design requires that the
menu system have no for knowledge of the content and
that it be tolerent of same being far less than valid.
While in the context in which you use the script you have total control
over the contents of the menu, as would anyone else using it.
Richard.
