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

Remove event_subscriber declaration from event_bus.h #42679

Merged
merged 1 commit into from
Aug 4, 2020

Conversation

kevingranade
Copy link
Member

Summary

SUMMARY: None

Purpose of change

Continuing to try to improve build times.

Describe the solution

Isolate the event_subscriber declaration from the other event declarations so forward includes are sufficient for declaring new subscribers.

Describe alternatives you've considered

pimpl-ing the magic class, pimpling the character class.

Testing

Needs to build.
Expecting CBA to show a mild improvement in character.h overhead, in the neighbourhood of 40s

@kevingranade kevingranade added [C++] Changes (can be) made in C++. Previously named `Code` Code: Build Issues regarding different builds and build environments labels Aug 3, 2020
@kevingranade kevingranade force-pushed the kevingranade-trim-event-include-overhead branch 2 times, most recently from 308e02d to d96e42a Compare August 3, 2020 21:52
@kevingranade kevingranade force-pushed the kevingranade-trim-event-include-overhead branch from d96e42a to bc9a51e Compare August 3, 2020 22:09
@anothersimulacrum
Copy link
Member

anothersimulacrum commented Aug 3, 2020

 94992 ms: src/character.h (included 142 times, avg 668 ms), included via:
  talker_avatar.o avatar.h  (1007 ms)
  timed_event.o avatar.h  (992 ms)
  character_crafting.o  (987 ms)
  action.o avatar.h  (987 ms)
  fungal_effects.o avatar.h  (982 ms)
  melee.o avatar.h  (979 ms)
  ...

56988 ms: ../src/character.h (included 62 times, avg 919 ms), included via:
  reading_test.o avatar.h  (1133 ms)
  itemname_test.o  (1097 ms)
  comestible_test.o  (1078 ms)
  item_location_test.o  (1069 ms)
  player_test.o  (1056 ms)
  active_item_test.o avatar.h  (1048 ms)
  ...

Looks like #42680 has a greater impact, but I'm not sure how much of that is actually relevant.

 90783 ms: src/character.h (included 142 times, avg 639 ms), included via:
  talker_npc.o avatar.h  (945 ms)
  monexamine.o avatar.h  (943 ms)
  talker_avatar.o avatar.h  (935 ms)
  melee.o avatar.h  (929 ms)
  map_field.o avatar.h  (927 ms)
  activity_item_handling.o avatar.h  (922 ms)
  ...
53129 ms: ../src/character.h (included 62 times, avg 856 ms), included via:
  item_location_test.o  (1008 ms)
  item_tname_test.o  (1004 ms)
  itemname_test.o  (1002 ms)
  comestible_test.o  (994 ms)
  vehicle_split_test.o  (986 ms)
  iuse_test.o avatar.h  (966 ms)
  ...

@ZhilkinSerg ZhilkinSerg merged commit d3e4f33 into master Aug 4, 2020
@kevingranade kevingranade deleted the kevingranade-trim-event-include-overhead branch August 24, 2020 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Build Issues regarding different builds and build environments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants