James Nelson has posted comments on this change.
Change subject: Prevent dev mode breakage when lots of jso classes are used.
......................................................................
Patch Set 3: Code-Review-1
(3 comments)
This patch doesn't actually fix the underlying issue; the code I uploaded
had errors, but even after fixing them and passing the related test,
inheriting elemental still causes JavaScriptObject$ to swell to over 80,000
members and overflow the integer used to hold class+memberId.
....................................................
File dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
Line 343: return (classId << 16) | memberId;
Yes, this is the _actual_ issue; though JavaScriptObject$ is already up to
80,000 items, so it's conceivable that 17 won't be enough for any
jsni-heavy libraries that take elemental as well.
There's a couple of places I've seen already that use (id & 0xffff) to
differentiate between class and member id...
Since this is dev mode, couldn't we just upgrade to long and give memberId
and classId each 25 bits? So long as we keep the value under 2^53 (max
floating point in js), it should be safe to pass them into js as doubles,
pull them out as long, and avoid overflow completely.
....................................................
File dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java
Line 41: private HashMap<Integer, Member> memberByMemberId;
Ya, ArrayList is preferable, this review was cobbled together from a
different patch, and I screwed it up. :-/
Line 85: int id = memberById.indexOf(m);
This method is only called once per DispatchClassInfo object, during
lazyInitTargetMembers(); we should, rightly so, make the check against the
the name->to->int map (and use the array list as originally designed).
Also, embarassingly enough, this code in fact, does not compile. I can
upload a fixed copy that will pass DispatchClassInfoTest, but will still
die as soon as Elemental is inherited.
The real problem isn't anything to do with the collections used here, but
rather it is in the use of a single integer to contain both class id and
member id; as soon as we pull in elemental,
com.google.gwt.core.client.JavaScriptObject$ has 80,000+ members in it (all
jso methods), so when we create a dispId of
(clsId << 16 | memberId), the memberId overflows into classId, causing
indexOOB and indeterminism galore.
The class that blows up most regularly is PotentialElement, and its
dispatch map is full of elemental methods. It can be seen reliably by a)
inheriting elemental.Elemental, then b) setting a breakpoint on line 177 of
c.g.g.dev.shell.Jsni (at:
member = dispatchInfo.getClassInfoByDispId(dispId).getMember(dispId);
You will have to continue about ten times before PotentialElement shows up,
but when it does, you can trace through and see how packing two 16 bit
integers together causes the overflow (expecting clsId=10, getting 11).
Since it's unlikely (I guess) that we can change the use of int dispId to
long dispId, the only workable alternative is to make
DispatchClassInfoOracle return classId and memberId separately, or to put
in special workarounds for members of JavaScriptObject$ (as no other object
should have more than 0xfff members).
--
To view, visit https://gwt-review.googlesource.com/2210
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c703d592556c500e338f95469b2db13f8024627
Gerrit-PatchSet: 3
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: James Nelson <ja...@wetheinter.net>
Gerrit-Reviewer: Brian Slesinsky <skybr...@google.com>
Gerrit-Reviewer: James Nelson <ja...@wetheinter.net>
Gerrit-Reviewer: John Ahlroos <j...@vaadin.com>
Gerrit-Reviewer: Thomas Broyer <t.bro...@gmail.com>
Gerrit-HasComments: Yes
--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "Google Web Toolkit Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
