[fluid-dev] Again on "Compile time constant lookup tables"

[Carlo Bramini]

Hello friends,

I was wondering what you think about using an external tool for compiling the const tables, like Python. Few lines on code into the CMakeLists.txt, no extra options, no compile time macros. After creating those tables, the compilation of the C code will work on all C compilers of the world. The tool will be searched by cmake with REQUIRED option. If we would like to avoid that, we may add a test for testing the presence of the generated files inside a specific directory, but perhaps it is not worth to do that extra work: in my opinion, having Python (or something else if you want) just at compile time is much less restrictive than having GLIB dependency at runtime.

what do you think?

Sincerely.

[Marcus Weseloh]

Hi,

Am So., 16. Sep. 2018 um 11:10 Uhr schrieb Carlo Bramini <

Post by Carlo Bramini
I was wondering what you think about using an external tool for compiling
the const tables, like Python.

In my opinion, that would be a simple, easy and straight forward solution.
And I would go one step further: make the generated files part of the
source distribution. That way you don't need Python to compile Fluidsynth.
Only if and when those tables need to be regenerated is Python a
requirement. And judging from the past years of development, the need to
change the tables won't come up very often.

Just think of the const tables as "configuration" and the Python script as
a tool to write that configuration. Similar to autotools configure scripts.
They get checked in to VCS all the time.

All the best,

Marcus

[Tom M.]

what you think about using an external tool for compiling the const tables, like Python. [...] The tool will be searched by cmake with REQUIRED option

I don't want to compel people to require yet another build dependency. While we have very good reasons to make glib a dependency, making e.g. python a dependency seems questionable as the current runtime solution works flawlessly for 99% of all users.

Carlo initially brought this idea up, in order to allow fluidsynth to run on very ressource limited hardware. I still support this and tried to find a solution that allows potentially more users to benefit from this... it turned out a little more complicated than I thought.

Personally I prefer the current C90 compliant solution that relies on the preprocessor (#423). The only significant drawback of this approach I see is that it does not allow to make *all* lookup tables "constexpr". Namely all tables that use pow(). As for pow() one needs a recursion, which the preprocessor is lacking.

If this is a problem (dont know, pls. tell me), I would vote for falling back to Marcus proposal and VC those autogenerated files generated by an external (upstream exclusive) tool. Yes one shouldn't do that, but watching the prior github discussion, we don't have any better alternatives currently; and yes, the lookup tables are (partly) given by the sf2 spec and very unlikely to ever receive a different implementation (i.e. values). In this case I'd vote for using Carlo's C tool... not python.

Tom

[Marcus Weseloh]

Hi,

Post by Tom M.
I don't want to compel people to require yet another build dependency.

While we have very good reasons to make glib a dependency, making e.g.
python a dependency seems questionable as the current runtime solution
works flawlessly for 99% of all users.

Post by Tom M.
[..]
Personally I prefer the current C90 compliant solution that relies on the

preprocessor (#423).

As you say: the current code works for 99% of all users and the need to
save a few KB of RAM is very rare indeed. Adding large chunks of fairly
complicated macros to the codebase just for that very rare use-case is not
really worth it, I think.

The approach in #423 already includes a compile-time switch for the
constant tables. If we simply replace the macro approach with a much
simpler and more readable Python script to generate the code, keep the
compile-time switch and have it disabled by default, then Python is only a
build dependency for the 1% of users (probably even less than 1%...) that
need constant tables. And my guess is that people with such special
requirements have no problem installing a Python interpreter. :-)

Cheers,

Marcus

[Tom M.]

the need to save a few KB of RAM is very rare indeed.

Yet, I would prefer if upstream takes care of this topic. I dont want to push responsibility to someone else.

Also, I prefer Carlo's C tool as it allows to use fluidsynths internal calculation routines to generate the constexpr tables. Whereas a python script would probably duplicate the existing implementation.

I see the following two solutions:

1) Use the Preprocessor (#423)
+ fully automatic C90 compliant approach
- huge memory requirements during compilation (>2GiB)
- doesnt work for all lookup tables
- MSVC wont work at all

2) Generate tables once and version control them
+ works for everone
+ works for all lookup tables
- slighly more maintaining effort
- "pollutes" the repository if tables are ever to change (highly unlikely though)

Opinions? Votes?

Tom

[Carlo Bramini]

Hello!

well, I did some experiments with the solution using Python interpreter.
It is not so bad afterall.
I have not code duplication here because the C implementation for calculating the tables is gone. Actually, in this solution, there is no need to keep it, unless an "hybrid" code is wanted and you want to build Fluidsynth without using Python.
The C code just include the generated header and nothing else, although I did it only for fluid_rvoice_dsp.c, probably it won't be much different in other places.
Changing the size of the tables will happen only in the python scripts, because into the C code you can get those sizes by using sizeof().
Perhaps it could also be an alternative to evaluate.

Another alternative could be to reject all the solutions and keep current code: they are simple and fast calculations for a decent hardware, I found that there are some look up tables that never change when the synthesizer is running and I felt the challange to find a solution for calculating them in some way at compile time, but actually it not the most important task.

Sincerely.

Post by Tom M.

the need to save a few KB of RAM is very rare indeed.

Yet, I would prefer if upstream takes care of this topic. I dont want to push responsibility to someone else.
Also, I prefer Carlo's C tool as it allows to use fluidsynths internal calculation routines to generate the constexpr tables. Whereas a python script would probably duplicate the existing implementation.
1) Use the Preprocessor (#423)
* fully automatic C90 compliant approach
* huge memory requirements during compilation (>2GiB)
* doesnt work for all lookup tables
* MSVC wont work at all
2) Generate tables once and version control them
* works for everone
* works for all lookup tables
* slighly more maintaining effort
* "pollutes" the repository if tables are ever to change (highly unlikely though)
Opinions? Votes?
Tom
_______________________________________________
fluid-dev mailing list
https://lists.nongnu.org/mailman/listinfo/fluid-dev

[Marcus Weseloh]

Post by Tom M.
Also, I prefer Carlo's C tool as it allows to use fluidsynths internal

calculation routines to generate the constexpr tables. Whereas a python
script would probably duplicate the existing implementation.

That's a good point. And if I remember correctly, then the idea of
compiling it and using it to generate the tables during the normal build
process was dropped because of cross-compiling problems. Maybe there's a
way around that... I did some tests with ExternalProject_add and got to the
point where, if I add a sub-directory to the source tree containing a small
c program and a very simple CMakeLists.txt, the program in the
sub-directory is compiled with the host compiler, the rest with the
cross-compiler.

I've added the following to the end of the root CMakeLists.txt:

include(ExternalProject)
ExternalProject_Add(gentables
DOWNLOAD_COMMAND ""
SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/src/gentables
CONFIGURE_COMMAND cmake <SOURCE_DIR>
INSTALL_COMMAND ""
BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/gentables
)
add_dependencies(libfluidsynth-OBJ gentables)

My cmake knowledge is virtually non-existant, I got the idea from here:
http://cmake.3232098.n2.nabble.com/Cross-compilation-and-native-executables-tp4463100p4483097.html

But maybe from here it could be possible to get cmake to execute the
compiled tool to generate the files and place them in the fluidsynth source
tree?

Cheers,

Marcus

[Marcus Weseloh]

Hi again,

I created a quick proof-of-concept branch here:
https://github.com/mawe42/fluidsynth/tree/gentables-extproj

It compiles fine both natively and with my cross-compiling system
(buildroot). But buildroot doesn't use toolchain files but sets a lot of
environment variables to get cmake to find the right toolchain. Not sure if
and how it would work with a different cross-compiling approach.

Cheers,

Marcus

[Tom M.]

Carlo: I have no code duplication here because the C implementation for calculating the tables is gone.

Removing the C implementation means that python would become a required build dependency, which is what I want to avoid, because, again, there is no reason to do so.

Marcus: Maybe there's a way around that... I did some tests with ExternalProject_add

Haven't heard of ExternalProject_Add() myself yet. Sounds viable. I think this in combination with Carlo's C tool can do the job.

Tom

[sqweek E.]

Post by Tom M.

Carlo: I have no code duplication here because the C implementation for calculating the tables is gone.

Removing the C implementation means that python would become a required build dependency, which is what I want to avoid, because, again, there is no reason to do so.

Committing the generated code would nicely solve this. It’s a pretty clean way to prevent a build time dependency for users who don’t want to fiddle with the code imo.

-sqweek

[Carlo Bramini]

Hello everyone,

if somebody is interested, I did a PR with Python support some days ago. It has resolved all issues on my side, it worked on embedded ARM, on MSVC versions 6 and 2017, on mingw-w64 and on unix-like cygwin. Although I understand that it cannot be accepted, perhaps it may be useful to other people.

Sincerely,

Post by sqweek E.

Post by Tom M.

Carlo: I have no code duplication here because the C implementation for calculating the tables is gone.

Removing the C implementation means that python would become a required build dependency, which is what I want to avoid, because, again, there is no reason to do so.

Committing the generated code would nicely solve this. It’s a pretty clean way to prevent a build time dependency for users who don’t want to fiddle with the code imo.
-sqweek
_______________________________________________
fluid-dev mailing list
https://lists.nongnu.org/mailman/listinfo/fluid-dev

[Tom M.]

I just opened a pull request based on Marcus proposal to use ExternalProject_Add(), that incorporates the changes discussed so far:

* generating lookup tables during compilation on-the-fly (not VC them)
* does not require any user intervention
* no additional build dependencies
* should work for cross compiling

This proposal is probably incomplete. Esp. the cross compilation part needs testing. I have had success when doing this for Android. But there are probably more cross compilation use-cases. Anybody interested, please have a look and feel free to give feedback:

https://github.com/FluidSynth/fluidsynth/pull/438

Tom

[Ralf Mattes]

Post by Marcus Weseloh
Hi,
Am So., 16. Sep. 2018 um 11:10 Uhr schrieb Carlo Bramini <

Post by Carlo Bramini
I was wondering what you think about using an external tool for compiling
the const tables, like Python.

+1

Post by Marcus Weseloh
In my opinion, that would be a simple, easy and straight forward solution.
And I would go one step further: make the generated files part of the
source distribution. That way you don't need Python to compile Fluidsynth.

That would be a 'bad idea'. Golden rule of development: never VC generated files.

Post by Marcus Weseloh
Only if and when those tables need to be regenerated is Python a
requirement. And judging from the past years of development, the need to
change the tables won't come up very often.
Just think of the const tables as "configuration" and the Python script as
a tool to write that configuration. Similar to autotools configure scripts.
They get checked in to VCS all the time.

??? That's not how the autotool chain is supposed to work, they clearly document that
the original input should be vesion controlled, not the results of running automake/autoconf et al.
The generated files should be included in _release tarballs_, but that's something completely different.

Cheers, RalfD

Post by Marcus Weseloh
All the best,
Marcus

[Marcus Weseloh]

Am Sonntag, 16. September 2018 11:43 CEST, Marcus Weseloh
Just think of the const tables as "configuration" and the Python script as

Post by Marcus Weseloh
a tool to write that configuration. Similar to autotools configure

scripts.

Post by Marcus Weseloh
They get checked in to VCS all the time.

??? That's not how the autotool chain is supposed to work, they clearly document that
the original input should be vesion controlled, not the results of running
automake/autoconf et al.
The generated files should be included in _release tarballs_, but that's
something completely different.

Think of the Python script as "autoscan" and the generated const tables as "
configure.in". Ok, granted... configure.in also gets modified after
generation.

"Golden rules" shouldn't be dogmas, I think. I fully agree that
auto-generated files should't be checked in to version control if their
contents might depend on when and where they are generated. But if I
understood the problem correctly, the const tables are exactly that: tables
of constant values. The Python script is expected to produce *exactly* the
same output no matter which system the script runs on.

Cheers,

Marcus

[Ceresa Jean-Jacques]

Hi Carlo,

 

First thanks for this usefull work.

I think that the reason of using of python script instead of a C program is not obvious for a lot of people, particularly for those who doesn't use cross compiler tools chain very often.

Please, may i suggest you add some comments about these reasons in a readme file (or  in gen_rvoice_dsp.py and gen_conv.py ).

Regards.


jjc


 

 

 

 

Message du 28/09/18 10:40
De : "Carlo Bramini"
A : "FluidSynth mailing list"
Objet : Re: [fluid-dev] Again on "Compile time constant lookup tables"