[gwt-contrib] Re: Devmode broken with elemental (#7481) (issue1801804)

[mdempsky]

http://gwt-code-reviews.appspot.com/1801804/diff/1/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java
File core/src/com/google/gwt/dev/shell/DispatchClassInfo.java (right):

http://gwt-code-reviews.appspot.com/1801804/diff/1/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java#newcode86
core/src/com/google/gwt/dev/shell/DispatchClassInfo.java:86: Integer id
= memberIdByMember.get(m);
Is this a very heavily used function and/or does memberBy[Member]Id get
very large normally?  If not, would it be simpler to just do:

  int id = memberById.indexOf(m);
  if (id == -1) {
id = memberById.size();
memberById.add(m);
  }
  memberIdByName.put(StringInterner.get().intern(name), id);

http://gwt-code-reviews.appspot.com/1801804/diff/1/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java#newcode90
core/src/com/google/gwt/dev/shell/DispatchClassInfo.java:90:
memberIdByName.put(StringInterner.get().intern(name), id);
I know this was preexisting practice, but why are we interning strings
here?  It doesn't seem to serve any purpose except to use use
interned-strings storage.

http://gwt-code-reviews.appspot.com/1801804/diff/1/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java#newcode237
core/src/com/google/gwt/dev/shell/DispatchClassInfo.java:237:
memberByMemberId = new HashMapInteger, Member(32767);
Why 32767?  Won't it size up appropriately, or will the table very
frequently have 32767 members?

http://gwt-code-reviews.appspot.com/1801804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Devmode broken with elemental (#7481) (issue1801804)

[jat]

http://gwt-code-reviews.appspot.com/1801804/diff/1/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java
File core/src/com/google/gwt/dev/shell/DispatchClassInfo.java (right):

http://gwt-code-reviews.appspot.com/1801804/diff/1/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java#newcode86
core/src/com/google/gwt/dev/shell/DispatchClassInfo.java:86: Integer id
= memberIdByMember.get(m);
On 2013/01/15 01:38:37, mdempsky wrote:

Is this a very heavily used function and/or does memberBy[Member]Id

get very

large normally?  If not, would it be simpler to just do:



   int id = memberById.indexOf(m);
   if (id == -1) {
 id = memberById.size();
 memberById.add(m);
   }
   memberIdByName.put(StringInterner.get().intern(name), id);


I believe that was essentially the implementation I originally wrote,
and was changed as part of a modification for performance and memory
usage (which is where the StringInterner calls came in).  At the time,
AdWords was having trouble running DevMode in a 32bit JVM.

I wasn't involved in that, but I presume there were lots of duplicate
strings which led to this being a win.

http://gwt-code-reviews.appspot.com/1801804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Devmode broken with elemental (#7481) (issue1801804)

[James]

Added a fix I needed on top of this patch for an unrelated issue trying
to access ::class fields in jsni.  I know it should be in its own issue,
but until this patch goes through, it will just make merging confusing.


http://gwt-code-reviews.appspot.com/1801804/diff/1/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java
File core/src/com/google/gwt/dev/shell/DispatchClassInfo.java (left):

http://gwt-code-reviews.appspot.com/1801804/diff/1/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java#oldcode237
core/src/com/google/gwt/dev/shell/DispatchClassInfo.java:237: cls,
true);
Instead of sending true to findMostDerived, can we send cls !=
Class.class; if you try to use @com.foo.SomeClass::class in jsni, dev
mode will blow up (security exception trying to make Class constructor
accessible).

I would open a new code review for it, but until this is fixed in trunk,
there's little sense in fixing it until this patch goes through.

http://gwt-code-reviews.appspot.com/1801804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors