Rhino: optimization for NativeFunction.java
Date:
Mon, 07 May 2001 14:19:59 +0200
From:
Igor Bukanov <igor.bukanov@windriver.com>
Organization:
Wind River
To:
Norris Boyd <nboyd@atg.com>
Hi, Norris!
This is the first of 3 patches that are completly independent from each
other.
Currently in NativeFunction its name stored as the first element in the
names array. But this lead to creation of a single element array for
each FunctionObject and for each script function that does not have
arguments or variables. The attached patch splits NativeFunction names
into simple functionName and argNames arrays and adjust code elsewhere
accordingly. This patch can increase memory footprint for anonymous
script functions without arguments because it adds additional field to
each NativeFunction, but I do not think this is a case to worry about.
Regards, Igor
Date: Mon, 07 May 2001 14:25:34 +0200
From: Igor Bukanov <igor.bukanov@windriver.com>
Organization: Wind River
To: Norris Boyd <nboyd@atg.com>
The current code that implements execMethod/methodArity for IdFunction
support returns an arbitrary value for id that is not known. This is not
very good behavior because it may hide bugs in the id support and it
also does not allow to distinguish ids that are used for function from
ids used for properties like String.length.
To fix this I changed semantic of the methodArity method to return -1
for an unknown/non-method id and added code to throw an exception for
bad ids. This change requires to adjust all NativeSomething objects that
use IdScriptabl and after a release all such interface changes would be
no go, but is not a release yet, right?
I also eliminated the "IdFunction f" argument from
IdFunction.Master.methodArity and the tagId field from IdFunction. When
I wrote the initial code for IdFunction.java, I added that just to be
able to use same id number in a class that implements IdFunction.Master
and its descendants via checking idTag. But that does not work in
general because IdScriptable can use id for non-function fields as well
so to avoid id clashes another way should be used. For example, if
someone would like to extend NativeMath to support more functionality,
he can use:
class Math2: extends NativeMath {
private static idBase;
{
if (idBase == 0) idBase = super.getMaximumId();
}
public int methodArity(int methodId) {
switch (methodId - idBase) {
case Id_foo: return 2;
case Id_bar: return 3;
}
return super.methodArity(methodId);
}
public Object execMethod
(int methodId, IdFunction f,
Context cx, Scriptable scope, Scriptable thisObj, Object[] args)
throws JavaScriptException
{
switch (methodId - idBase) {
case Id_foo: return ...;
case Id_bar: return ...;
}
return super.execMethod(methodId, f, cx, scope, thisObj, args);
}
protected int getMaximumId() { return idBase + MAX_ID; }
protected String getIdName(int id) {
switch (id - idBase) {
case Id_foo: return "for";
case Id_bar: return "bar";
}
return super.getIdName(id);
}
...
private static final int
Id_foo = 1,
Id_bar = 2,
MAX_ID = 2;
etc.
Note that a simpler solution to make MAX_ID field public in NativeMath
and write in Math2:
private static final int
Id_foo = NativeMath.MAX_ID + 1,
Id_bar = NativeMath.MAX_ID + 2,
MAX_ID = NativeMath.MAX_ID + 2;
does not work because in this way adding any new id to NativeMath would
break binary compatibility with Math2.
Rhino: fix for race conditions in listeners code in Context.java
Date:
Mon, 07 May 2001 14:22:57 +0200
From:
Igor Bukanov <igor.bukanov@windriver.com>
Organization:
Wind River
To:
Norris Boyd <nboyd@atg.com>
The current code for listeners and contextListeners in Context.java is
not race condition free. If contextListeners Vector would be modified
during context event firing loops, the code can produce
index-out-of-bounds exception. The problem with listeners array is more
subbtle and comes from the fact that ListenerCollection.java uses code like:
for(Enumeration enum = getAllListeners();enum.hasMoreElements();) {
Object listener = enum.nextElement();
if(iface.isInstance(listener)) {
array.addElement(listener);
}
}
where getAllListeners() uses Vector.elements to get element enumeration.
But to work with such enumeration in a thread safe way, one has to
synchronized against Vector, otherwise between enum.hasMoreElements()
and enum.nextElement() the last element can be removed.
Initially I thought to fix ListenerCollection and use it for
contextListeners as well, but then I realized that in its current form
ListenerCollection is very inefficient (it produces too many objects
just to get simple array to fire events), so I wrote ListenerArray.java
and use it in Context.java. It makes life simpler and shrinks code as well.
Subject:
rhino bug(s)
Date:
Mon, 30 Apr 2001 23:07:00 -0700
From:
Mike Dixon <MDixon@placeware.com>
To:
nboyd@atg.com
hi. i'm a happy rhino user, and just stumbled across what looks like a
pretty basic bug in the property stuff on ScriptableObject... (i'm running
1.5, but it looks like this code hasn't changed in CVS.) since it looks
like you're actively developing (even though it's been a while since
1.5...) i figured you might be interested -- apologies if i missed a more
formal bug reporting process...
the symptom was that i got a "Hashtable internal error" thrown from
getSlotToSet. reading the code, here's what i think could happen:
- create a new object (slots.length is initially 5)
- add 3 properties
- delete those 3 properties
(now count == 0, and slots[i] == REMOVED for 3 values of i)
- add 2 more properties
now assume that you're unlucky, and that these two hash to different values
than the first three; now you have 2 elements of slots[] containing real
slots, and the other three containing REMOVED.
now what happens when you try to create another slot? getSlotToSet is only
willing to put something in a null slot[], and you haven't got one, so you
get the internal error.
writing this message encouraged me to try to write a test case to reproduce
it, and in fact it's trivial:
js> x={}; x.a=x.b=x.c=1; delete x.a; delete x.b; delete x.c; x.d=x.e=1
1
js> x.whatever=1
(boom)
by the way, while reading the code i also noticed what looks like another,
less consequential bug: addSlot increments count before deciding to grow
the table, which is done with a recursive call, which will cause count to
be incremented again -- right? as far as i can tell, setting count too big
will only cause it to grow the table a little early next time, so it
doesn't really matter, but it looks wrong.
.mike.
mozilla/js/rhino/org is now distributed between
mozilla/js/rhino/src and mozilla/js/rhino/toolsrc.
The build.xml has been split in three.
Docs now live in the project directory.
These changes mean that the cvs directories mirror the distribution and thus a distribution
will build the same way as a cvs build.
I attach optimization patch for NativeDate that makes all js... methods
private, removes passing of unnecessary parameters and replaces
checkInstance by realThis call with false as the third parameter.
Regards, Igor
Hi, Norris!
Here is another small optimization for NativeDate in DayFromMonth method
where it replaces arrays by few ifs.
Regards, Igor
Rhino: patch for IdScriptable.java and question about useDynamicScope
Date:
Mon, 16 Apr 2001 17:55:19 +0200
From:
Igor Bukanov <igor.bukanov@windriver.com>
Organization:
Wind River
To:
Norris Boyd <nboyd@atg.com>
Hi, Norris!
Here is a patch to IdScriptable.java that fixes sealed semantic and
makes Something.prototype.constructor to behave just as having DONTENUM
attribute, not DONTENUM|READONLY|PERMANENT. It also renames
seal_function to sealFunctions.
I also have a following question. I added nextInstanceCheck to
IdScriptable.java and its usage in realThis in NativeDate to emulate
code from FunctionObject where it looks up prototype in search for
NativeSomething instance if useDynamicScope is true. But could you
describe why it is necessary? I can understand why doing something like
var proto = new Date();
function Test() { }
Test.prototype = proto;
var test = new Test();
print(test.getDay()); // same as proto.getDay()
would be useful in ceratain situations, but what it has to do with
shared scopes?
Regards, Igor
Minor fix to JSDebugger
Date:
Wed, 28 Mar 2001 16:34:24 -0800
From:
Christopher Oliver <coliver@mminternet.com>
Organization:
Primary Interface LLC
To:
nboyd@atg.com
Hi Norris,
Attached is a minor fix to the JSDebugger GUI that causes the tool-bar buttons to all have the same width.
I checked out and modified a file from CVS today. See the screenshot below.
Cheers,
Chris
