CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.26k stars 4.12k forks source link

Makefile: JSON linting should be parallelizable #21661

Closed Maykeye closed 6 years ago

Maykeye commented 7 years ago

Expected Behavior

If I run make -j16, on my Ubuntu 16.04 lint.cache step still processes files one by one. Which is slow. VERY SLOW.

Actual behavior

1 thread only for every file, it takes about 1 minute to complete everything.

Steps to reproduce the behavior

For easiness of demonstration, replace lint and lint.cache rule with

  LINT_JSON_WHITELIST=$(shell awk '/^[^\#]/ { print $$1 }' json_whitelist)
  lint: $(LINT_JSON_WHITELIST) | $(ODIR)
      @echo $(LINT_JSON_WHITELIST) | xargs  -n1 -P$(J) echo perl tools/format/format.pl -cqv&
      @echo $(LINT_JSON_WHITELIST) | xargs  -n1 -P$(J)      perl tools/format/format.pl -cqv

(First echo for debugging purposes)

now run time make J=1 -B lint. On my machine it'll report real 0m56.791s (like current rule from master branch)

run time make J=16 -B lint, it'll report real 0m6.119s. MUCH better.

Possible solutions

1) Use black magic to generate rules on the fly for each .json file and touch which file was processed, i.e. create obj/data/mods/Medieval_Stuff/tools.json_linted to specify when file tools.json was linted.

Pros: very in-spirit of make: files will be linted only when they are changed. make -j will be respected. Cons: complex realization. In partially because there is a fun with wildcards:

consider these lines in json_whitelist

data/mods/*/modinfo.json
data/mods/blazemod/*.json

expanding it leads to two instances of data/mods/blazemod/modinfo.json because it fits both these wildcards. Not taking it into account will lead to

  Makefile:983: warning: overriding recipe for target 'data/mods/BrightNights/modinfo.json_linted'
  Makefile:983: warning: ignoring old recipe for target 'data/mods/BrightNights/modinfo.json_linted'

2) Use xargs and add yet another variable (like GOLD=, LUA=) Pros: simple realization Cons: it's already take a long time to type make RELEASE=1 GOLD=1 LUA=1

3) Use black magic to extract -J argument if there was, and use it for xargs. Pros: -j will be respected as it should be Cons: very dirty and OS specific

kevingranade commented 6 years ago

Linter has been replaced, the new one should be much faster than the old one was.

kevingranade commented 6 years ago

Your first option is the only really workable option, AFAIK if you pass the value of the -j flag to xargs, you'll end up exceeding your requested parallelism since make will only count it as a single task.