gridcoin-community / Gridcoin-Research

Gridcoin-Research
MIT License
585 stars 173 forks source link

Move code from main.h into separate source files #195

Open denravonska opened 7 years ago

denravonska commented 7 years ago

The compilation times are really, really long even when building on a powerful machine. Part of this problem can be found in main.h where a lot of the code is located. In order for almost any file to build they have to include main.h in one way or another, so each file will take a very long time to build due to the sheer code size, and changing one file tend to trigger a massive rebuild dependency chain.

In Bitcoin they have moved code into separate files (see https://github.com/bitcoin/bitcoin/tree/master/src/primitives). This helps structuring the program, reduces the build time for each file and reduces the necessity to rebuild files when making changes.

grctest commented 7 years ago

In Bitcoin they have moved code into separate files

At what version did they begin this practice?

denravonska commented 7 years ago

The oldest commit for primitives/block.h is bitcoin/bitcoin@effc2770f50554416e7d7b27f5c5b5cef9489440, so it's been a while.

denravonska commented 7 years ago

My initial plan was to cherry pick the commits from the Bitcoin repo and eventually we would end up close to where they are now. However, too much have changed besides the code extraction for this to be viable. Instead, I'll manually move the code so our structure is more in line with how it looks in Bitcoin now.

TheCharlatan commented 7 years ago

I would suggest to merge in whatever Bitcoin improvement was merged into Blackcoin after the code was forked from it.

denravonska commented 7 years ago

None of the coins (including Blackcoin) have followed the Bitcoin source tree so I guess we can forget about this issue (move code and reduce build times) as all of them seem to dump their code into the headers.

We do have to make a choice of which code base we want as a sibling though; Bitcoin or a fork? As it stands now I think the Bitcoin source is out since it's way too different. Is Blackcoin the most reasonable candidate?

grctest commented 7 years ago

Blackcoin Codebase Upgrades

Many altcoins still sit on top of a very old bitcoin codebase (0.9). This stifles progress. I'm no developer so can't go into great technical detail, but I am aware for some time Janko33 has been bringing BlackCoin up to date, and currently the 0.12.1 base is almost complete. This was done based on work rat4 started to get the 0.11 codebase into Blackcoin and Janko33 picked up where he left off. There will obviously be the need for some testing when Janko33 feels it is ready for that and I'm sure would welcome more people to try it out (Join Slack!). There are lots of advantages of having recent bitcoin commits also in blackcoin, I'm sure Janko can go into more technical detail, but his work with Bitcore (above) was only possible because of this base work.

I think we should follow the Bitcoin codebase (not including segwit), however both Peercoin and Blackcoin commit backlogs could provide an indication of what Bitcoin commits are of high priority.

denravonska commented 7 years ago

Maybe if we can isolate instead of integrating the Gridcoin specific code it could be easier to start from scratch; that is migrating our specific code to a fresh tree? I'm not too familiar with the trees but it's going to be a whole lot of work just to stay in sync with Blackcoin (which is the closest code sibling), not to mention Bitcoin.

Just brainstorming here :)

grctest commented 7 years ago

Migrating our code to a fresh tree does sound like far less effort than implementing each commit on an individual basis. By doing so, we'd be one of the few POS coins which have an up-to-date codebase compared to Bitcoin.

@gridcoin Thoughts? Rebasing vs Integrating backlog of commits?

gridcoin commented 7 years ago

@GRCTest, Excuse me but I did not know you were a programmer. What background do you have in development to be qualified to discuss this?

@Denravonska, Question: Are you sure that compiling a change takes a long time when main.h does not change? On Windows, I can compile almost any change quickly, as long as one of the static header files does not change. First let us know if the behavior on linux is different. We are both compiling with Mingw 32 correct?

TheCharlatan commented 7 years ago

@gridcoin On Linux I also get minimal compilation times, for small changes, so I do not think that the compilation time is a major issue. However since there is a lot of code in main, chances are it will be changed often and therefore result in long compilation times again. So breaking it up will minimize the amount of changes on the main header and the times it needs to recompiled. Also it would of course increase readability and introduce a more modular form to the code.

denravonska commented 7 years ago

@gridcoin @TheCharlatan It could be that I've been operating in "global" code that I've seen the frequent rebuilds enough for it to be an issue. The way I see it there are two problems: a lot of the code is in the headers and because of this a lot of headers have to include a lot of headers, causing a large dependency chain. 94% of the source in the headless wallet has serialize.h in its dependency chain even though very few files actually need to serialize.

All of this header code and dependency chains add up. For example, script.cpp takes 14.7 seconds to compile on an Athlon64 and 17.4 seconds on a 2GHz Xeon. With optimizations turned off (-O2 is 21.7s on the Athlon). That is a lot for a 2000 line file :( Some other random files:

Maybe I'm nitpicking but I believe it's healthy to have as short of a build cycle as possible. If we can reduce the time it takes to compile one file we should, especially since the solution does not affect the final output. All it takes is for someone to just do it.

@grctest: Keep discussing it. The more thought going into it the better regardless of the outcome.

denravonska commented 7 years ago

We can also use precompiled headers to reduce build times. As a quick test on script.cpp the build time drops from 14.7 to 8.9 seconds once the header has been generated.

For the entire headless wallet the -O0 build on my machine goes down from 5m54s to 4m59s. It's nothing radical but it could be a neat quality-of-life fix that requires no source code modifications other than removing two unused GetDifficulty declarations.

Edit: Scratch that. While precompiled headers do reduce the overall build time they also increase the source volatility. Touching any header would require a full rebuild.

grctest commented 7 years ago

Quoting @gridcoin:


Quoting @grctest : "@gridcoin Thoughts? Rebasing vs Integrating backlog of commits?"


"@GRCTest, Excuse me but I did not know you were a programmer. What background do you have in development to be qualified to discuss this?"


I'm not a c++ programmer, however there are many additional supporting roles within the software development process (change management for example, of which I have IRL experience of). As far as I'm aware there has been no mandate to implement a technocracy within the GRC development process/governance.

By answering the original question "Rebasing vs Integrating backlog of commits?", you will focus development towards a common goal which will likely be sooner than your initial estimate of 01-MAR-2018 (especially with the recent creation of the gridcoin-community org). Additionally, if we plan on individually cherry-picking commits, I'll need to dedicate more time towards improving the commit backlog spreadsheet.

denravonska commented 7 years ago

Regarding that I think it might be easier to ignore the Bitcoin stuff and just try to clean up after ourselves. If the Gridcoin code wasn't nuked into the Bitcoin code it would be easier to transition, but we'd have to take baby steps and focus on one function at a time. A great way to get unit tests going though.

So, this can probably be closed and a new "Move Gridcoin code out of bitcoin" can be created.

jamescowens commented 3 years ago

We have done the transaction. part. The block. part is yet to be done.