Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extremely long compilation time for testing #75

Open
mourner opened this issue Sep 18, 2019 · 8 comments
Open

Extremely long compilation time for testing #75

mourner opened this issue Sep 18, 2019 · 8 comments

Comments

@mourner
Copy link
Member

mourner commented Sep 18, 2019

Currently make takes WAY too long to compile everything (5–10 minutes for a Travis/Appveyor job), and almost all the time is spent compiling fixture objects, one for each target (bench, tests, viz).

Is there anything we could do to speed this up? E.g. maybe convert fixtures to header-only? I'm not too good with C++ so any help would be appreciated. Iterating on Earcut.hpp is pretty painful with compile times this long.

@springmeyer
Copy link
Contributor

I agree - these compile times are awful and oddly bad. I'm not familiar with the c++ code to offer a concrete recommendation for a re-write, but I'm sure there is likely a reasonable one to radically reduce the compile times. But short of that a potential workaround is to use ccache. If major changes happen to earcut.hpp this may not help much. But it will help massively if only minor changes are made to the test files. For example, a compile for me locally is several minutes but only 11 seconds once ccache is used.

@springmeyer
Copy link
Contributor

Working on this in #77

@mrgreywater
Copy link
Collaborator

As alternative we could load the test cases at runtime from json files, but that would require to add a json library as additional dependency.

@mourner
Copy link
Member Author

mourner commented Sep 18, 2019

@mrgreywater or maybe we should convert the fixtures to regular text files to avoid the json dependency?

@mrgreywater
Copy link
Collaborator

mrgreywater commented Sep 18, 2019

Sure, we could use some easy custom file format and use a minimal custom parser, but we would then need to convert all fixtures whenever earcut.js is updated, which could be otherwise avoided. Also manually adding some fixtures for testing would likely be more of a hassle with a minimal parser.

Some minor inconveniences of using files at runtime are also

  1. Some cross platform glue to iterate over fixture folder (or use std::filesystem but only with c++17)
  2. Having to either copy the fixtures to the same folder as the output binary, or having to set the working directory (will lead to an error if you just double click the executable).

@mourner
Copy link
Member Author

mourner commented Sep 18, 2019

but we would then need to convert all fixtures whenever earcut.js is updated, which could be otherwise avoided.

@mrgreywater (edit) I think using https://github.com/mapbox/earcut.hpp/blob/master/test/convert_tests.js isn't much of a hassle, and we wouldn't be changing the status quo on that by switching the format.

BTW, would it be possible to make all fixtures header-only?

@mrgreywater
Copy link
Collaborator

mrgreywater commented Sep 18, 2019

I know we're converting the tests right now, with json that could be skipped though - assuming we add the libtess deviation to the json files in earcut.js .

BTW, would it be possible to make all fixtures header-only?

Making stuff header-only only increases compilation time for the benefit of having less files and clutter to deal with. Also I don't think we can realistically make them header-only. They would then have to be included in some cpp file to be compiled, which means you would have to manually list all fixtures in a cpp which defeats the purpose of the current system.

@mourner
Copy link
Member Author

mourner commented Sep 18, 2019

assuming we add the libtess deviation to the json files in earcut.js

I don't want to care much about libtess in Earcut on the JS side, so I'd prefer to keep them here.

Making stuff header-only only increases compilation time for the benefit of having less files and clutter to deal with.

In this case, even the simplest test fixture with a few points takes A TON of time to compile — likely because all the baggage in the headers needs to be compiled again and again for each one. Having one file to compile would avoid the duplication, right?

I'm fine with manually listing the fixtures as long as I can compile the thing in seconds rather than 5 minutes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants