pret / pokecrystal

Disassembly of Pokémon Crystal
https://pret.github.io/pokecrystal/
2.08k stars 784 forks source link

Many smaller .o files, kept in their own build directory #425

Open roukaour opened 6 years ago

roukaour commented 6 years ago

e.g. objects/ or temp/

Probably there are also more self-contained subsystems that can get their own .o files to improve build time.

yenatch commented 6 years ago

build/pokecrystal/ and build/pokecrystal11/, or just build/

roukaour commented 6 years ago

Note that if .o files aren't next to corresponding .asm files, this will no longer work:

$(foreach obj, $(crystal11_obj), $(eval $(call DEP,$(obj),$(obj:11.o=.asm))))
$(foreach obj, $(crystal_obj), $(eval $(call DEP,$(obj),$(obj:.o=.asm))))
yenatch commented 6 years ago

i imagine it would work the same as pokeruby where subdirs are created in build/pokecrystal/

yenatch commented 6 years ago

or build/crystal//build/crystal11/

mid-kid commented 6 years ago

If we could create all build output in a build/ directory (i.e. gfx files), that'd be great.

Rangi42 commented 6 years ago

What if the code mentioned a build directory, e.g.:

INCBIN "build/gfx/player/chris_back.2bpp.lz"

And then the Makefile rules were changed to map build/X to X? Like:

build/%.2bpp: %.png
    $(RGBGFX) $(rgbgfx) -o $@ $<
    $(if $(tools/gfx),\
        tools/gfx $(tools/gfx) -o $@ $@)

It would also need @mkdir -p $(dirname $^) since build/gfx/player/ doesn't exist yet.

This has the advantage of the current system, that you don't need an explicit list of built files and their dependencies, plus the advantage of not cluttering source directories with build products.

yenatch commented 6 years ago

mkdir -p in recipes would dramatically slow down the build. doing it prebuild works fine though

the disadvantage is it's confusing, but i guess it already is confusing to include a different file than you put in

Rangi42 commented 6 years ago

pokegold-spaceworld has a working solution for this.

rawr51919 commented 4 years ago

Could said solution be adapted to work here too? Surely it wouldn't take too much effort.

mid-kid commented 4 years ago

No, because everything is in monolithic sections and INCLUDEing all of the constants in every single build unit is extremely slow. See #631

rawr51919 commented 2 years ago

What if the code mentioned a build directory, e.g.:

INCBIN "build/gfx/player/chris_back.2bpp.lz"

And then the Makefile rules were changed to map build/X to X? Like:

build/%.2bpp: %.png
  $(RGBGFX) $(rgbgfx) -o $@ $<
  $(if $(tools/gfx),\
      tools/gfx $(tools/gfx) -o $@ $@)

It would also need @mkdir -p $(dirname $^) since build/gfx/player/ doesn't exist yet.

This has the advantage of the current system, that you don't need an explicit list of built files and their dependencies, plus the advantage of not cluttering source directories with build products.

This solution might work much better and not be so damn massive of a refactor compared to mid-kid's work, perhaps doing the mkdir on the entire build directory structure before starting the build might be the way to go with this one as yenatch once specified. When I have time I could make a PR for this specific method

vulcandth commented 1 year ago

No, because everything is in monolithic sections and INCLUDEing all of the constants in every single build unit is extremely slow. See #631

Sorry to necro this topic... Is this due to how make does it's dependency checking? Would https://github.com/gbdev/rgbds/pull/1043 help in this case?

mid-kid commented 1 year ago

It wouldn't be very different than adding INCLUDE "everything.inc" at the start of every file. In splitting I had reformatted every file to only INCLUDE the files it needed directly, and some included files would include other files recursively depending on what they needed (for example the many files that need macros/enum.inc). It used include guards to prevent double includes, just like in C.

Back then doing it this way gave at least a 4x speedup in clean rebuilds over the INCLUDE "everything.inc" solution. However, rgbds has changed a lot over the years since, and has gotten significant improvements in terms of performance, so this should be re-evaluated. Personally I still believe including files individually has benefits over everything.inc in preventing name conflicts. For example, you don't need the map script macros or pokemon and move constants in audio scripts, and vice versa. I had an audio_common.inc and events_common.inc for both of these kinds of scripts, so maybe there's a balance to be struck, but that's a conversation for a different day.

As an aside, preinclude is a nice feature but I almost prefer having an INCLUDE "everything.inc" in every file, even if only to cover the cases where you specifically don't want everything to be included, without having to modify the makefile.

Rangi42 commented 1 year ago

I think --preinclude is worth considering for the current architecture where we just have a few top-level .o files listed in the Makefile, but with splitting, yeah, manual includes are more clear.