Ticket #866 (closed RFC: done)

Opened 5 years ago

Last modified 4 years ago

DEPRECATE (after review) the *bigint*, pow* and nextkey_keyed VTABLE functions

Reported by: bacek Owned by:
Priority: normal Milestone:
Component: core Version: 1.3.0
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description (last modified by cotto) (diff)

Current nomenclature of VTABLEs is bloated and requires cleanup. The following functions can be removed at any time post-1.0:

get_bigint
set_bigint_*
pow
i_pow
nextkey_keyed*

-- Bacek

Change History

  Changed 5 years ago by allison

nextkey_keyed* I can see, if they're no longer needed for iterators (they never were a good way to implement iterators anyway). I don't see the reasoning for bigint and pow, could you explain your thoughts?

  Changed 5 years ago by bacek

Just as cleanup of rare used code.

1. get_bignum is just shortcut for $P0 = new 'BigNum'; set $P0, orig. And only one "real" usage is inside Integer PMC.

2. set_bignum_* isn't used anywhere in Parrot. And not exposed in ops.

3. pow/i_pow for consistency sake. Why we have VTABLE_pow, but not "exp", "ln", etc?

-- Bacek

follow-up: ↓ 4   Changed 5 years ago by allison

1. Except that it's impossible to retrieve a bignum value from another PMC without get_bignum, because an N register can't hold the value.

3. The same could be argued for all the math vtable functions, but you have to draw the line somewhere. Simple "to the power of X" is a common operation, exp and ln aren't.

nextkey_keyed* can go, but the bignum and pow vtable functions stay.

in reply to: ↑ 3   Changed 5 years ago by bacek

Replying to allison:

1. Except that it's impossible to retrieve a bignum value from another PMC without get_bignum, because an N register can't hold the value.

Ok. Let's add "get_complex". For exactly same reason...

Agree (but dislike it) with math ops.

There is more vtables to review:

  • set_bignum_int (which is not used at all.)

  • get_repr (which have notice in pdd17 about redundancy with get_string)
  • assing_string_native/set_string_native looks pretty duplicates.
  • "string" versions of bitwise operators. Do we really need them? How often they are used. Can we hust replace them with METHODs?

*ADD* more VTABLEs for consistency sake:

  • i_logical_or, _and, _xor

-- Bacek

  Changed 5 years ago by bacek

set_*_same VTABLE functions were removed in r41229.

  Changed 5 years ago by cotto

  • description modified (diff)
  • summary changed from DEPRECATE (after review) some VTABLE functions to DEPRECATE (after review) the *bigint*, pow* and nextkey_keyed VTABLE functions

  Changed 5 years ago by coke

The last update here seems to contradict the only review comment (by allison).

Can we get some documentation on the update? Thanks.

  Changed 5 years ago by cotto

nextkey_keyed was removed in r40193.

As for Coke's comment, that was a substantial failure on my part for not carefully reading the ticket. Clearly more discussion is needed before the bignum and pow VTABLE functions are removed, if ever.

  Changed 5 years ago by cotto

Given allison's input and the fact that the nextkey_keyed VTABLE function was removed, I'll close this ticket in a few day unless there are any objections.

  Changed 5 years ago by whiteknight

I have a few objections.

get_bignum seems to me like the wrong solution to the problem. Allison says "it's impossible to retrieve a bignum value from another PMC without get_bignum", but that doesn't make any sense to me. We're talking about a situation where one PMC contains a "big" value, and we need to move that value to another PMC, and we both PMCs in this transaction aren't the same type (so we can't just do an assign or a set_pmc). We don't have any such situation in Parrot core that would satisfy this need.

Not to mention bacek's objection that, for consistency's sake, we don't have get_complex or even get_bigint. And even looking beyond mathematics types, we don't have a way to get_callsignature or get_hash from a PMC either: Why do we need a VTABLE interface to extract one particular type of PMC value from an arbitrary PMC type? Alternatively, what if an HLL is using a different PMC type like DecNumber instead of BigNum? Where's the get_decnumber VTABLE?

The usefulness of get_bignum is extremely limited, and the operation could be easily satisfied using any number of other interfaces:

$P0 = new ['BigNum']
$P0 = $P1

...Where $P1 is any arbitrary PMC type that might contain a large numerical value. In this case, the logic to extract a big-formatted number from an arbitrary PMC type is properly encapsulated in the BigNum PMC, and not spread out into all types throughout the repo AND all types defined in all other projects that run on Parrot. An alternative interface could be:

$P0 = $P1.'get_bignum'()

But the beauty here is that the 'get_bignum' method doesn't need to be defined up-front, it could be part of a role mixed in at runtime. This has the benefit that we could just as easily mixin 'get_complex', 'get_bigint', 'get_decnum' methods to other types too.

Since most types in the repo do not contain large-formatted number values in any case, by far the most common interface would be something like this:

$P0 = new ['BigNum']
$N0 = $P1
$P0 = $N0

...And this wouldn't be much slower or harder than what happens now.

Most types do not ever make use of large-formatted numbers anyway, the get_bignum and set_bignum VTABLEs are mostly worthless. Looking now, I only see three types that implement get_bignum: Integer, String, and BigInt. And notably I can't find any good way to get a BigInt value from a BigNum PMC. I guess for that conversion, you would need to export the one to a string and then load the string into the BigInt. It's notable that the Float PMC doesn't even provide a get_bignum VTABLE. Getting a BigNum from an Integer easily goes like this:

$P0 = ['BigNum']
$I0 = $P1
$P0 = $I0

Similarly, from a String:

$P0 = ['BigNum']
$S0 = $P1
$P0 = $S0

...and all the logic to do the conversion is magically encapsulated into BigNum.

In summary:

  • Most types don't use large-formatted numerical values in any case, so we CAN use I and N registers to move numerical values between almost all PMC types
  • The get_bignum and set_bignum VTABLEs are extremely rarely used and for most applications do nothing but take up valuable space
  • There are two large-format number types in core, BigInt and BigNum. We also have a viable add-on module in the DecNumber library that adds another type. There could be more. These types should encapsulate their own conversion logic and not spread it out. We don't want get_bigint and get_decnum VTABLEs, but we're setting an example that this is the way to deal with these kinds of types.
  • There are strong incongruities with other mathematics types like Complex (no get/set_complex), and all other arbitrary PMC types (no get/set_callsignature). Hell, there isn't even a dedicated VTABLE to retrive an Integer, Float, or String from an arbitrary PMC. It makes no sense that all PMCs should have a VTABLE method to retrieve a BigNum but not to retrieve these other types directly.

  Changed 5 years ago by whiteknight

I got sidetracked and didn't talk about how lousy the pow_* vtables are either. By my count we have 6 such vtables: pow, pow_int, pow_float, i_pow, i_pow_int, and i_pow_float. Considering how infrequently used any pow() operations, it seems absurd that every single VTABLE structure for every single PMC needs to have 6 pointers reserved to it.

We have a small handful of types that implement any of the pow variants: Complex, Integer, BigNum, Scalar, BigInt, and default. default redirects all requests to MMD, Integer and Scalar define all variants following a template similar to the following:

VTABLE void i_pow_float(FLOATVAL value) {
    VTABLE_set_number_native(INTERP, SELF,
        pow(SELF.get_number(), value));
}

If all we need is a thin wrapper around libc's pow() function, we could easily provide that in an op or even a runtime loadable library function. There's no reason to have 6 VTABLEs that are not defined in most types and where they are defined they mostly fall back to libc's pow() function.

This also raises an incongruity where we are missing such treatment of other oft-unused libc functions like log(), log10(), ln(), sin(), cos(), etc. The same reason why we don't want to add 6 VTABLEs for each of these operations is the exact same reason why we shouldn't have them for pow().

That said, there are some types that do make legitimate use of pow: Complex, BigNum, and BigInt. However I would suggest that all these types should encapsulate the necessary behavior in methods and not require all PMC types to allocate storage for pointers to pow functions when most PMCs won't use them.

follow-up: ↓ 13   Changed 5 years ago by allison

For the record: whiteknight's reasoning is persuasive and I'm now in favor deprecating pow & bignum. The list of relevant vtable functions:

PMC* get_bignum()
void set_bignum_int(INTVAL value)
void set_bignum_num(FLOATVAL value)
void set_bignum_str(STRING* value)
PMC* pow(PMC* value, PMC* dest)
PMC* pow_int(INTVAL value, PMC* dest)
PMC* pow_float(FLOATVAL value, PMC* dest)
void i_pow(PMC* value) :write
void i_pow_int(INTVAL value) :write
void i_pow_float(FLOATVAL value) :write

in reply to: ↑ 12   Changed 5 years ago by jkeenan

Replying to allison:

For the record: whiteknight's reasoning is persuasive and I'm now in favor deprecating pow & bignum. The list of relevant vtable functions:

Have the listed functions actually been deprecated? Do we have any plan for removing them once they are deprecated?

Thank you very much.

kid51

  Changed 4 years ago by bacek

  • status changed from new to closed
  • resolution set to done

All mentioned vtables were removed. Closing ticket.

Note: See TracTickets for help on using tickets.