Add kernel implementation FACTORIAL_INT of Factorial

[fingolfin]

This is an alternative to PR #1968. Some micro-benchmark results:

gap> range:=[1..10];;N:=100000;;
gap> for i in [1..N] do l:=List(range,Factorial); od; time;
499
gap> for i in [1..N] do l:=List(range,factorial_frank); od; time;
226
gap> for i in [1..N] do l:=List(range,FACTORIAL_INT); od; time;
171

gap> range:=[1..30];;N:=100000;;
gap> for i in [1..N] do l:=List(range,Factorial); od; time;
2401
gap> for i in [1..N] do l:=List(range,factorial_frank); od; time;
1275
gap> for i in [1..N] do l:=List(range,FACTORIAL_INT); od; time;
505

gap> Factorial(1000000);; time;
518
gap> factorial_frank(1000000);; time;
327
gap> FACTORIAL_INT(1000000);; time;
187

[codecov]

Codecov Report

Merging #1969 into master will increase coverage by 0.31%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master    #1969      +/-   ##
==========================================
+ Coverage    65.2%   65.52%   +0.31%     
==========================================
  Files         930      930              
  Lines      275034   275887     +853     
  Branches    12718    12768      +50     
==========================================
+ Hits       179346   180774    +1428     
+ Misses      92401    92321      -80     
+ Partials     3287     2792     -495

Impacted Files	Coverage Δ
lib/combinat.gi	66.35% <ø> (-0.19%)	⬇️
src/integer.c	87.67% <80%> (+1.1%)	⬆️
extern/gmp/mpn/generic/toom43_mul.c	96.77% <0%> (-3.23%)	⬇️
src/system.c	54.02% <0%> (-1.85%)	⬇️
src/saveload.c	56.1% <0%> (-1.61%)	⬇️
src/iostream.c	49.05% <0%> (-1.34%)	⬇️
src/records.c	66.05% <0%> (-0.78%)	⬇️
src/vector.c	92.16% <0%> (-0.4%)	⬇️
src/sortbase.h	85.54% <0%> (-0.35%)	⬇️
src/io.c	54.76% <0%> (-0.28%)	⬇️
... and 89 more

[frankluebeck]

Shouldn't this be Factorial instead of FactorialInt (which does not exist on GAP level)?

And why is this test made in a separate function?

Why not changing FactorialInt above into FuncFactorial and removing this FuncFACTORIAL_INT and the InstallGlobalFunction(Factorial, FACTORIAL_INT); line in the library?

[fingolfin]

Yes, that should be Factorial and not FactorialInt.

The check is in a separate function because so far all those integer functions had a "kernel version" (i.e. FactorialInt), and a "library version (i.e. FuncFACTORIAL_INT) -- in other words, blind habit. But its unlikely that we will want to directly call Factorial from the kernel, so that's overkill here, so I agree we can merge the two together.

(These remarks apply similarly to Binomial and a few others, I'll look into tweaking those, too)

[fingolfin]

The reason why I did not remove InstallGlobalFunction(Factorial, FACTORIAL_INT); is that then I also need to remove the Declare* counterpart, and decide where to move the GAPDoc comment; and I'd rather keep the change as small as possible.

[frankluebeck]

The performance seems significantly better than that of the function in PR #1968 for all reasonable inputs. So this PR should be preferred.

[fingolfin]

Updated.