Possible bugs?
Charles Oliver Nutter
1. EnumSet bug:
# dalvikvm -classpath ruboto.jar org.jruby.Main -e "puts 1"
86
HugeEnumSet.java:102:in `next': java.lang.ArrayIndexOutOfBoundsException
from HugeEnumSet.java:52:in `next'
from Ruby.java:1237:in `initErrno'
The "86" there is the size of the enumset, so it doesn't appear to be an
issue with it being empty. I haven't looked at Dalvik source yet
(assuming this is available?). I fixed it by switching to an array of
enums (Errno.values() in this case) rather than using an EnumSet (which
was unnecessary anyway).
2. The BufferedReader/BufferedInputStream warnings were annoying but
easy to fix.
3. The errors for exceeding -Xss are totally useless. Most common seems
to be NullPointerException at totally bizarre locations.
4. Trying to import android.content.Context blows up somewhere in
reflection. A few other java.* classes I tried seem ok:
irb(main):001:0> import 'android.content.Context'
Class.java:-2:in `getDeclaredMethods': java.lang.NoSuchMethodException
from ClassCache.java:137:in `getDeclaredMethods'
from Class.java:666:in `getDeclaredMethods'
from JavaClass.java:1738:in `getMethods'
from JavaClass.java:718:in `setupClassMethods'
from JavaClass.java:509:in `initializeClass'
from JavaClass.java:453:in `<init>'
from JavaClass.java:894:in `createJavaClass'
This kills my ability to do eye-popping demos of scripting Android
classes from Ruby, and therefore makes me sad.
All my modifications up to this point are on JRuby trunk and about to be
released in JRuby 1.2 RC1.
- Charlie
[1] http://blog.headius.com/2009/02/domo-arigato-mr-ruboto.html
Mark Murphy
> A couple items while I've been working on Ruboto[1]:
Maybe others can chime in on the specific errors, since they're kinda
out of my area of expertise, but here's a couple of quick comments:
> I haven't looked at Dalvik source yet
> (assuming this is available?).
Dalvik's source can be found here:
http://android.git.kernel.org/?p=platform/dalvik.git;a=tree
> 4. Trying to import android.content.Context blows up somewhere in
> reflection. A few other java.* classes I tried seem ok:
>
> irb(main):001:0> import 'android.content.Context'
> Class.java:-2:in `getDeclaredMethods': java.lang.NoSuchMethodException
> from ClassCache.java:137:in `getDeclaredMethods'
> from Class.java:666:in `getDeclaredMethods'
> from JavaClass.java:1738:in `getMethods'
> from JavaClass.java:718:in `setupClassMethods'
> from JavaClass.java:509:in `initializeClass'
> from JavaClass.java:453:in `<init>'
> from JavaClass.java:894:in `createJavaClass'
FWIW, android.content.Context is an abstract class -- does that cause
any particular problems on the JRuby import?
--
Mark Murphy (a Commons Guy)
http://commonsware.com
_The Busy Coder's Guide to Android Development_ Version 2.0 Published!
Charles Oliver Nutter
> FWIW, android.content.Context is an abstract class -- does that cause
> any particular problems on the JRuby import?
It shouldn't; we just create a proxy representation of it and populate
it with the data we gather from reflection. I was hoping to get at it
and access the services it provides; obviously this issue stands in the
way of a lot of things.
I will have a look at the Android source as well as JRuby, but it
certainly seems like Android's fault so far.
On a related question: if this were to get fixed, how frequently are
updates pushed to consumers?
- Charlie
fadden
wrote:
> to be NullPointerException at totally bizarre locations.
not seeing that?
If you set a debugger to break when an NPE is thrown, does it show a
null reference?
Charles Oliver Nutter
I did not see a StackOverflowError, and the NPEs were reporting line
numbers that did not match the compiled code. I checked it multiple
times, and finally just tried upping -Xss, which ultimately seemed to
fix it in that particular case.
I know I had some other peculiar error that wasn't StackOverflowError,
but I don't recall what it was.
- Charlie
Hannes Wallnoefer
wrote:
>
> reflection. A few other java.* classes I tried seem ok:
>
> irb(main):001:0> import 'android.content.Context'
> Class.java:-2:in `getDeclaredMethods': java.lang.NoSuchMethodException
> from ClassCache.java:137:in `getDeclaredMethods'
> from Class.java:666:in `getDeclaredMethods'
> from JavaClass.java:1738:in `getMethods'
> from JavaClass.java:718:in `setupClassMethods'
> from JavaClass.java:509:in `initializeClass'
> from JavaClass.java:453:in `<init>'
> from JavaClass.java:894:in `createJavaClass'
>
> This kills my ability to do eye-popping demos of scripting Android
> classes from Ruby, and therefore makes me sad.
run on Android SDK. A few observations that may be useful to locate
the problem:
- Reflections seems to work on the classes in the top level android
package, but is broken for every other android.subpackage.* class I've
tried:
js> new Packages.android.Manifest()
android.Manifest@4017cbf0
js> new Packages.android.R()
android.R@4017f750
js> new Packages.android.R.anim()
android.R$anim@401825b0
js> new Packages.android.app.Activity()
java.lang.NoSuchMethodException
at java.lang.Class.getDeclaredMethods(Native Method)
at java.lang.ClassCache.getDeclaredPublicMethods(ClassCache.java:150)
at java.lang.ClassCache.getDeclaredMethods(ClassCache.java:163)
at java.lang.ClassCache.getFullListOfMethods(ClassCache.java:233)
at java.lang.ClassCache.getFullListOfMethods(ClassCache.java:207)
at java.lang.ClassCache.getAllPublicMethods(ClassCache.java:188)
at java.lang.Class.getMethods(Class.java:864)
at org.mozilla.javascript.JavaMembers.discoverAccessibleMethods
(JavaMembers.java:380)
- But: Using reflection interactively in the rhino shell seems to work
for all android classes:
js> new java.lang.Object().getClass().getClassLoader().loadClass
("android.app.Activity").getDeclaredMethods()
[Ljava.lang.reflect.Method;@40174f00
I'm rather clueless as to what this means, bit it seems the java to
android bytecode translation is playing some role in it, since the
"pure dalvik vm" reflection seems to be working ok.
Hannes
Mark Murphy
> On a related question: if this were to get fixed, how frequently are
> updates pushed to consumers?
I'm not the best person to answer this question, since I'm not on the
core Android team, but, here goes:
Since the release of the T-Mobile G1, there have been two over-the-air
(OTA) updates that got a lot of discussion. These fixed bugs and added
some features. However, it is unclear if every bug fix gets updated this
way -- e.g., if somebody patched the bug you're experiencing, but the
bug is not noticeably affecting other device functionality, I don't know
that it will get shipped.
Major updates, such as oft-mentioned cupcake branch, are decidedly less
frequent. There is no official statement on frequency, but it feels like
it might wind up being 9-12 months. Again, that's just a guess.
All updates have to go through the device manufacturer (and, for OTA,
the carrier) as well. Once there are a half-dozen major manufacturers
and lots of little guys, I imagine the release timeframe for updates
(major and minor) will get fuzzier still.
My recommendation is that you put bugs in the issue tracker
(http://b.android.com). You will then have a shot at finding out which
Android release addresses them. You can use that to advise
JRuby-on-Android developers (e.g., gotta be running such-and-so release
of Android or newer).
fadden
wrote:
> numbers that did not match the compiled code. I checked it multiple
> times, and finally just tried upping -Xss, which ultimately seemed to
> fix it in that particular case.
reproduce this situation?
The default stack size is 8KB, so I'm not surprised the stack
overflowed, but the behavior should be rather more deterministic. ;-)
Dan Bornstein
<charles...@sun.com> wrote:
> 2. The BufferedReader/BufferedInputStream warnings were annoying but easy to
> fix.
In early testing we found that there was a lot of code that blithely
used the default Buffered* constructors, and it turned out to cause
excessive memory use. Rather than deviate from using what appeared to
be the de facto standard buffer size in the constructor and risk
subtly breaking legacy code that depended on the size (even though of
course nothing *should* depend on it), we opted instead to issue a
warning. And then we set about altering all the uses of these default
constructors in Android code to instead be explicit (and typically
choose a smaller buffer size).
This was my decision, so feel free to direct your annoyance in my
general direction.
With regard to the other issues, and emphasizing what I think others
have already said, bug reports with explicit steps-to-reproduce that
can be independently compiled and run are the gold standard and give
us the best chance of being able to track down and fix the issues.
Thanks in advance for putting them together.
And thanks for being on the bleeding edge of Android in general!
-dan
Charles Oliver Nutter
> In early testing we found that there was a lot of code that blithely
> used the default Buffered* constructors, and it turned out to cause
> excessive memory use. Rather than deviate from using what appeared to
> be the de facto standard buffer size in the constructor and risk
> subtly breaking legacy code that depended on the size (even though of
> course nothing *should* depend on it), we opted instead to issue a
> warning. And then we set about altering all the uses of these default
> constructors in Android code to instead be explicit (and typically
> choose a smaller buffer size).
Well, here's my counterpoint: I just went through and set them all to
8192 anyway, since this is the same code used for every other JRuby app.
Am I wrong to have done so? Are others going to be wrong when they do so?
I can appreciate wanting to reduce memory, but this would be better left
as a translation-to-dex warning or a tell-me-how-I-can-improve-please
mode than as an always-present runtime warning, and you'd save the extra
check and logging for apps that don't bother fixing it. Because I
suspect, as in my case, most folks will do little more than hardcode the
default 8192 size anyway.
Finally...those streams have default constructors for a reason, and
everyone uses them. Warning because people use the default constructor
is exceeding the contract of those types.
> With regard to the other issues, and emphasizing what I think others
> have already said, bug reports with explicit steps-to-reproduce that
> can be independently compiled and run are the gold standard and give
> us the best chance of being able to track down and fix the issues.
> Thanks in advance for putting them together.
I'll try to reduce them and file issues this week.
- Charlie
Dan Bornstein
wrote:
> > In early testing we found that there was a lot of code that blithely
> > used the default Buffered* constructors, and it turned out to cause
> > excessive memory use. Rather than deviate from using what appeared to
> > be the de facto standard buffer size in the constructor and risk
> > subtly breaking legacy code that depended on the size (even though of
> > course nothing *should* depend on it), we opted instead to issue a
> > warning. And then we set about altering all the uses of these default
> > constructors in Android code to instead be explicit (and typically
> > choose a smaller buffer size).
>
> Well, here's my counterpoint: I just went through and set them all to
> 8192 anyway, since this is the same code used for every other JRuby app.
> Am I wrong to have done so? Are others going to be wrong when they do so?
the same of others, without context.
However it absolutely was the case that these constructors were being
improperly used in Android, and this was in fact a source of excessive
memory consumption and overactive gc. And, very saliently, it was very
easy to inadvertently cause these problems, because developers never
got timely feedback that anything was even potentially problematic. We
discovered the problems during detailed profiling, but it would have
been much better had we never had to do the profiling in the first
place. (Well, of course we still profile stuff, but it would have been
nice to have one less problem to concern ourselves with up-front.)
If in fact 8k is right for you in the context of Android, then please
use 8k buffers! And that speaks to why we didn't change the default
behavior. If 8k happened to be right for you no matter whether it's on
Android or not, and we were to go and "secretly" change the buffer
size to (say) 256 bytes, then you could rightfully claim that we
messed up your perfectly working code. We don't want to mess up your
code, but that default constructor really can be an "attractive
nuisance."
Hence, the message. It doesn't change the programmatic behavior of
your code, but it does serve to warn folks of a known pitfall that we
have already seen cause real problems. So, we ask you to be explicit,
in the hope that that means you've actually considered the use case in
context and are therefore making a conscious choice, not simply
disregarding the issue without thought.
Warm regards,
-dan
Charles Oliver Nutter
> If in fact 8k is right for you in the context of Android, then please
> use 8k buffers! And that speaks to why we didn't change the default
> behavior. If 8k happened to be right for you no matter whether it's on
> Android or not, and we were to go and "secretly" change the buffer
> size to (say) 256 bytes, then you could rightfully claim that we
> messed up your perfectly working code. We don't want to mess up your
> code, but that default constructor really can be an "attractive
> nuisance."
Well, it's your platform, you can do what you want :) I would not have
chosen to rap developers on the knuckles for doing what they do on a
desktop Java platform. I would have changed the default buffer size,
since nowhere in the specification of BufferedInputStream et al does it
say there's a specific size behind the scenes. Developers depending on
the default buffer size being 8192 are much more at fault than
developers using a default constructor because they presumably don't
care, and yet you penalize the latter group. Indeed, the default
constructor should be expected to do what's best on a given platform, so
changing it to be a smaller buffer size would seem to me totally acceptable.
> Hence, the message. It doesn't change the programmatic behavior of
> your code, but it does serve to warn folks of a known pitfall that we
> have already seen cause real problems. So, we ask you to be explicit,
> in the hope that that means you've actually considered the use case in
> context and are therefore making a conscious choice, not simply
> disregarding the issue without thought.
Then perhaps it could warn me exactly once? :) Is there a good need to
blast warnings every time I construct a buffered stream with a
*published* default constructor? It's basically a public, default
constructor I can't call. Doesn't that sound goofy?
- Charlie
Dan Bornstein
wrote:
> blast warnings every time I construct a buffered stream with a
> *published* default constructor? It's basically a public, default
> constructor I can't call. Doesn't that sound goofy?
won't go so far as to make it an actual error. I freely admit that
there are other ways the issue could have been finessed.
Frankly, in the long term I expect that the platform, and the hardware
it typically runs on, will have evolved enough that the message won't
make sense anymore, at which point we'll just remove it. (Or maybe
sooner, if I see too many torches and pitchforks at my door.)
-dan
Hannes Wallnoefer
>
> Dan Bornstein wrote:
>>
>> If in fact 8k is right for you in the context of Android, then please
>> use 8k buffers! And that speaks to why we didn't change the default
>> behavior. If 8k happened to be right for you no matter whether it's on
>> Android or not, and we were to go and "secretly" change the buffer
>> size to (say) 256 bytes, then you could rightfully claim that we
>> messed up your perfectly working code. We don't want to mess up your
>> code, but that default constructor really can be an "attractive
>> nuisance."
>
> Well, it's your platform, you can do what you want :) I would not have
> chosen to rap developers on the knuckles for doing what they do on a desktop
> Java platform. I would have changed the default buffer size, since nowhere
> in the specification of BufferedInputStream et al does it say there's a
> specific size behind the scenes. Developers depending on the default buffer
> size being 8192 are much more at fault than developers using a default
> constructor because they presumably don't care, and yet you penalize the
> latter group. Indeed, the default constructor should be expected to do
> what's best on a given platform, so changing it to be a smaller buffer size
> would seem to me totally acceptable.
I strongly agree with Charlie here. Arguing that changing the default
buffer size may have broken buggy code that may not even exist, but
then expecting developers to change perfectly fine existing code
(probably each and every java library that makes use of buffered IO)
seems backwards to me. It feels like you deprecated the zero-args
constructor instead of doing the sensible thing of making it work for
your vm and platform.
Hannes
Charles Oliver Nutter
I'd also play devil's advocate and say it wasn't *that* big an
annoyance. But I'd trade the annoyance for a smaller default buffer in a
heartbeat.
- Charlie
Dianne Hackborn
Dianne Hackborn
Android framework engineer
hac...@android.com
Note: please don't send private questions to me, as I don't have time to provide private support. All such questions should be posted on public forums, where I and others can see and answer them.
Charles Oliver Nutter
> What is the right default size?� It seems to me best to encourage
> developers to explicitly declare the size they want, since they are the
> only ones who actually know what is appropriate for them.
Encourage; do not force. There are better ways to encourage than
spraying warnings at runtime.
- Charlie
Romain Guy
Such as?
On Feb 25, 2009 7:13 PM, "Charles Oliver Nutter" <charles...@sun.com> wrote:Dianne Hackborn wrote: > > What is the right default size?� It seems to me best to encourage develo...
Charles Oliver Nutter
> Such as?
Spray warnings at build time :)
- Charlie
fadden
> On Feb 24, 11:46 pm, Charles Oliver Nutter <charles.nut...@sun.com>
> wrote:
>
> > I did not see a StackOverflowError, and the NPEs were reporting line
> > numbers that did not match the compiled code. I checked it multiple
> > times, and finally just tried upping -Xss, which ultimately seemed to
> > fix it in that particular case.
>
> That's pretty bizarre. Is there something I can download & run to
> reproduce this situation?
down. A bug in the ARM interpreter was causing the pending
StackOverflowError to be lost, so if you had a "finally" clause the
implicit re-throw at the end would generate a NullPointerException.
The exception would be attributed to a line of code in the "finally"
block.
Because of the way Dalvik deals with synchronized methods, you would
see something similar there; the NPE would likely be attributed to the
closing brace of the method.
Charles Oliver Nutter
> Somewhat by coincidence, I got a reproducible case and tracked it
> down. A bug in the ARM interpreter was causing the pending
> StackOverflowError to be lost, so if you had a "finally" clause the
> implicit re-throw at the end would generate a NullPointerException.
> The exception would be attributed to a line of code in the "finally"
> block.
>
> Because of the way Dalvik deals with synchronized methods, you would
> see something similar there; the NPE would likely be attributed to the
> closing brace of the method.
Bingo, that's exactly what I saw. It made no sense to me at all. Great find!
- Charlie