987281 - Refactor AuoLocalJNIFrame

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

AutoLocalJNIFrame could use some improvements

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Refactor AutoLocalJNIFrame (v1)

This patch changes the default number of frame entries from 128 to 15. The dalvik stack is very limited, and if we're using more than 15 local references in a JNI function, we might be doing something we shouldn't be doing. It also changes CheckForException() to use AndroidBridge::HandleUncaughtException() to make uncaught exceptions more noticeable.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 8395838 [details] [diff] [review]
Refactor AutoLocalJNIFrame (v1)

Review of attachment 8395838 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/android/AndroidBridge.h
@@ +516,5 @@
>  };
>  
>  class AutoLocalJNIFrame {
>  public:
> +    AutoLocalJNIFrame(int nEntries = 15)

why 15? (note: 15 is still more reasonable than 128)

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #2)
> 
> why 15? (note: 15 is still more reasonable than 128)

We ask for (nEntries + 1) entries to account for possible exception taking up one more spot, and 16 total slots seems like a nice power-of-2 number to use. FWIW, the Dalvik stack supports 512 total slots per thread.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/23f6bebf6b2e

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/23f6bebf6b2e