616254 - Add ability to determine runmode and features from within the avm shell

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Chris Peyer]

It would be useful to be able to determine the compiled features and the current runmode in order to test legacy bugs that only fail in certain modes.

Comment 1

[Chris Peyer]

Attachment: Add System.getAvmplusFeatures and System.getAvmplusRunmode

Not sure if its ok to hardcode the runmode values.

Comment 2

[Rick Reitmaier]

Would it be ok to assume a default state (say runmode=mixed) and then use System.getArgv() and some AS parsing magic to deduce the state?

Or even better if we're going to head in this direction how about making the interface more generic and provide a getEnv() call that provides this info in a large string block containing 'var=value' pairs, allowing us to easily pile-on whatever info we need.

"
version=xxx
build=yyy
features=mmm,nnn,aaa
runmode=mixed
some_interesting_info=foo-bar
"

Comment 3

[Rick Reitmaier]

Comment on attachment 494793 [details] [diff] [review]
Add System.getAvmplusFeatures and System.getAvmplusRunmode

r- until reply from comment above.

Comment 4

[Rick Reitmaier]

oh forgot to include the extract code:

  var a = System.getEnv()
  var mode = a.match(/^runmode\ *=\ *(\w+)/m)[1]

Comment 5

[Lars T Hansen]

Comment on attachment 494793 [details] [diff] [review]
Add System.getAvmplusFeatures and System.getAvmplusRunmode

FWIW I like this approach.

Comment 6

[Lars T Hansen]

(In reply to comment #2)
> Would it be ok to assume a default state (say runmode=mixed) and then use
> System.getArgv() and some AS parsing magic to deduce the state?

That seems like a bit of a hack, why would you do that?  API minimality?

> Or even better if we're going to head in this direction how about making the
> interface more generic and provide a getEnv() call that provides this info in
> a large string block containing 'var=value' pairs, allowing us to easily
> pile-on whatever info we need.

In that case it seems like the better approach would be to return a data structure insted of a string that has to be parsed, with all the complications that entails once values can start containing spaces (say).  After all that's what's implied by "pile-on" :-)

I can see the desire for something more comprehensive than what Chris is proposing but the method names he's choosing are pretty unambiguous, and this is just an avmshell API that we can change in the future.

Comment 7

[Edwin Smith]

(In reply to comment #1)
> Created attachment 494793 [details] [diff] [review]
> Add System.getAvmplusFeatures and System.getAvmplusRunmode
> 
> Not sure if its ok to hardcode the runmode values.

Do the API names need to contain the word Avmplus?

Question about requirements.  Should getAvmplusRunmode() return the *mode*, ie mixed/interp/jit, or the current execution mechanism (interp or jit)?  With OSR, the latter can change on the fly.  I think you want the former.

Over time, new modes may be added, and start getting policy parameters.

Comment 8

[Chris Peyer]

(In reply to comment #7)
> (In reply to comment #1)
> 
> Do the API names need to contain the word Avmplus?

No.  I'll update the patch to remove Avmplus - I was just following getAvmplusVersion()
> 
> Question about requirements.  Should getAvmplusRunmode() return the *mode*, ie
> mixed/interp/jit, or the current execution mechanism (interp or jit)?  With
> OSR, the latter can change on the fly.  I think you want the former.

Yep.  Former.  It returns what config.runmode has been set to.

Comment 9

[Chris Peyer]

Attachment: add getFeatures and getRunmode

Keeping the original two calls, but renamed to not have Avmplus in the method name.  I agree with Lars that if we are going to aggregate the data it would be preferable to be in an an object/dictionary instead of a string blob.  For now though these two methods cover everything I need and are pretty clear.  I'm not aware of any other info we need from within our tests.

Comment 10

[Tamarin Bot]

changeset: 5632:00830e7731c4
user:      Chris Peyer <[e-mail]>
summary:   Bug 616254: Add ability to determine runmode and features from within the avm shell (r=rreitmai)

http://hg.mozilla.org/tamarin-redux/rev/00830e7731c4