| Subject: | Better documentation |
Please write a more precise documentation.
This module just optimizes 2 old signature usecases, with the first lines in blocks
shifting from @_
my $arg = shift;
and using defaults
my $arg = shift // const;
The current documentation is misleading and overly broad.
It should also be documented that with the new OP_SIGNATURE in 5.22
these small optimizations (bypassing the stack) are then ignored.
The name is very misleading - Devel::OptPurpleSigs would have been a better name.
It will not be faster with 5.22 and davem's experimental shift assignment converter, it will rather be slower.
In contrast the similarily named Faster is really a general purpose optimizer.
BTW: The real performance problem with your old purple sigs is not the temp stack for the lexical assignments. This is neglectable.
It's the overly slow arity check, which I had to optimize at compile-time and the inability to access the args from the stack, bypassing @_, as a XSUB does it. This would be possibly with your custom ops, but should be better done with davem's OP_SIGNATURE and a few additions he doesn't know yet of, but can be applied on top of it. HASSIG e.g. is required to bypass @_ copies.
Out of a much larger patch (which does types, call-by-ref and proper sigs + protos together)
something like this, using only one costly run-time arity check for the general case.
- if (min_arity != 0) {
- initops = op_append_list(OP_LINESEQ,
- newSTATEOP(0, NULL,
- newLOGOP(OP_OR, 0,
- newBINOP(OP_GE, 0,
- scalar(newUNOP(OP_RV2AV, 0,
- newGVOP(OP_GV, 0, PL_defgv))),
- newSVOP(OP_CONST, 0, newSViv(min_arity))),
- op_convert_list(OP_DIE, 0,
- op_convert_list(OP_SPRINTF, 0,
- op_append_list(OP_LIST,
- newSVOP(OP_CONST, 0,
- newSVpvs("Too few arguments for subroutine at %s line %d.\n")),
- newSLICEOP(0,
- op_append_list(OP_LIST,
- newSVOP(OP_CONST, 0, newSViv(1)),
- newSVOP(OP_CONST, 0, newSViv(2))),
- newOP(OP_CALLER, 0))))))),
- initops);
- }
- if (max_arity != -1) {
- initops = op_append_list(OP_LINESEQ,
- newSTATEOP(0, NULL,
- newLOGOP(OP_OR, 0,
- newBINOP(OP_LE, 0,
- scalar(newUNOP(OP_RV2AV, 0,
- newGVOP(OP_GV, 0, PL_defgv))),
- newSVOP(OP_CONST, 0, newSViv(max_arity))),
- op_convert_list(OP_DIE, 0,
- op_convert_list(OP_SPRINTF, 0,
- op_append_list(OP_LIST,
- newSVOP(OP_CONST, 0,
- newSVpvs("Too many arguments for subroutine at %s line %d.\n")),
- newSLICEOP(0,
- op_append_list(OP_LIST,
- newSVOP(OP_CONST, 0, newSViv(1)),
- newSVOP(OP_CONST, 0, newSViv(2))),
- newOP(OP_CALLER, 0))))))),
- initops);
+ fail_op = op_convert_list(OP_DIE, 0,
+ op_convert_list(OP_SPRINTF, 0,
+ op_append_list(OP_LIST,
+ newSVOP(OP_CONST, 0,
+ newSVpvs("Wrong number of arguments for subroutine %s at %s line %d.\n")),
+ newSLICEOP(0,
+ op_append_list(OP_LIST,
+ newSVOP(OP_CONST, 0, newSViv(3)),
+ op_append_list(OP_LIST,
+ newSVOP(OP_CONST, 0, newSViv(1)),
+ newSVOP(OP_CONST, 0, newSViv(2)))),
+ op_convert_list(OP_LIST, 0,
+ newUNOP(OP_CALLER, 0,
+ newSVOP(OP_CONST, 0, newSViv(0))))))));
+ if (min_arity == max_arity) {
+ return op_append_list(OP_LINESEQ,
+ newSTATEOP(0, NULL,
+ newLOGOP(OP_OR, 0,
+ newBINOP(OP_EQ, 0,
+ scalar(newUNOP(OP_RV2AV, 0,
+ newGVOP(OP_GV, 0, PL_defgv))),
+ newSVOP(OP_CONST, 0, newSViv(min_arity))),
+ fail_op)),
+ initops);
+ }
+ else if (min_arity > 0) {
+ if (max_arity == -1)
+ return op_append_list(OP_LINESEQ,
+ newSTATEOP(0, NULL,
+ newLOGOP(OP_OR, 0,
+ newBINOP(OP_GE, 0,
+ scalar(newUNOP(OP_RV2AV, 0,
+ newGVOP(OP_GV, 0, PL_defgv))),
+ newSVOP(OP_CONST, 0, newSViv(min_arity))),
+ fail_op)),
+ initops);
+ else
+ return op_append_list(OP_LINESEQ,
+ newSTATEOP(0, NULL,
+ newLOGOP(OP_AND, 0,
+ newLOGOP(OP_OR, 0,
+ newBINOP(OP_LT, 0,
+ scalar(newUNOP(OP_RV2AV, 0,
+ newGVOP(OP_GV, 0, PL_defgv))),
+ newSVOP(OP_CONST, 0, newSViv(min_arity))),
+ newBINOP(OP_GT, 0,
+ scalar(newUNOP(OP_RV2AV, 0,
+ newGVOP(OP_GV, 0, PL_defgv))),
+ newSVOP(OP_CONST, 0, newSViv(max_arity)))),
+ fail_op)),
+ initops);
}
return initops;
}
--
Reini Urban