Thomas,
I'm a little knee deep in I/O slide stuff at the moment, so maybe you
can sanity check my thinking here. Let me describe what used to be
happening in the Json stuff and what I was trying to change it to before
2.5, but probably didn't finish.
Essentially, in ProdMode I wanted to run all unboxed values. This is
theoretically possible, but can't be made to work without 2 compiler
changes:
1) String is allowed as a JSO, so cast checks to cast String to
JsonString fail if -XdisableCastChecking is not on
2) JsonNumber and JsonBoolean, backed by primitives in prodmode, can
fail in conditional checks against != null/null.
To fix these, I started boxing values on fetch, but kept everything
stored raw so it could be consumed by other JSO code unchanged.
However I noticed the boxing/deboxing code produces significant bloat
from the getters, because they are inlined and much more frequent.
So a few weeks ago I decided to migrate to a strategy of 'box on store
and stay boxed', so Json.parse() and put/set calls box the primitives.
They are stay boxed unless one of the coercion functions is used.
For DevMode, constructing Json* objects gives you JRE implementations
for speed. However, because a DevMode function can still call a JSNI
function which returns a pure interface (e.g. JsonObject) both versions
can be active, and therefore, the code must be defensive for DevMode. I
haven't tried it, but I'm guessing DevMode handles boxed Number and
Boolean properly.
Ultimately, for ProdMode, maybe in a 2.5.1 release, I want to modify the
compiler to completely eliminate any boxing at all.
http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonArray.java
File elemental/src/elemental/js/json/JsJsonArray.java (right):
http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonArray.java#newcode44
elemental/src/elemental/js/json/JsJsonArray.java:44: return this[index];
On 2012/06/13 15:36:39, tbroyer wrote:
Should box() the value.
(JsJsonObject box()es its value in get(String))
Hmm, that's wrong then. I use to box on get, but it turns out to produce
inlined-box code everywhere that is bloated, so I changed it to
box-on-store in most places. I need to make sure this is consistent. It
turns out that in large apps, fetches are more frequent than
assignments, so the code is smaller.
http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonFactory.java
File elemental/src/elemental/js/json/JsJsonFactory.java (right):
http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonFactory.java#newcode70
elemental/src/elemental/js/json/JsJsonFactory.java:70: return
Object(value);
See my other comment. Based on analysis of internal apps, I decided to
opt to keep the values stored as boxed values instead of boxing on get,
since it leads to more bloated code.
If you don't box a primitive in prodmode, you end up with code like this
failing:
JsonValue x = foo.get("y");
if (x != null) {
// do something
}
If 'x' turns out to be unboxed boolean 'false' or unboxed numeric 0,
this null check fails because the compiler optimizes the if(x != null)
to if(x)
On 2012/06/13 15:36:39, tbroyer wrote:
Is that needed in prod mode? shouldn't JsJsonValue.box() be used here?
Actually, is that needed at all, given both JsJsonArray and
JsJsonObject 'box'
their values before returning them?
All that's needed is to box() the returned value, in case you parse a
'primitive' (e.g. parse("true"))
http://gwt-code-reviews.appspot.com/1728806/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
