-
Notifications
You must be signed in to change notification settings - Fork 2
Cycle breaking #234
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
Draft
Paul-Licameli
wants to merge
35
commits into
howsoai:main
Choose a base branch
from
Paul-Licameli:cycle-breaking
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Cycle breaking #234
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
... SBFDSColumnData is free from it, as are EvaluableNode, EvaluableNodeManagement, PerformanceProfiler, but those three are still in a small s.c.c.
... It is needed only there and this removes direct (but not transitive) dependency of EvaluableNodeTreeFunctions on Interpreter
... This frees Parser and EvaluableNodeTreeFunctions from the s.c.c but the new file EntityTreeFunctions remains in it
... This frees FileSupportCAML from the big s.c.c
... Because there is another struct LoadEntityStatus also in the global namespace used for the C api, which I don't want to change.
... so Entity does not depend directly on Interpreter, however EntityExecution becomes part of the big s.c.c
... so compiler assures that the .h file has no omitted, hidden dependencies
... before factoring it into two classes
... freeing it and GeneralizedDistance and EvaluableNodeTreeDifference from the big s.c.c.
... Freeing AssetManager, EntityExecution, EntityQueryBuilder, and Interpreter from cycles
... Conviction, EntityManipulation, EntityQueries, EntityQueryCaches, KnnCache, SeparableBoxFilterDataStore are free from cycles. Entity, EntityTreeFunctions, and EntityWriteListener remain in the s.c.c.
... which breaks up the last remaining s.c.c larger than one, in non-third-party code.
... No cycles broken, but some unnecessary edges removed
... so that EntityManipulation does not depend on DistanceReferencePair
... Ideally each source code file should be in some folder, which is boxed in the generated graphs, and the quotient graph of folders is acyclic.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Untangle the include dependency graph of Amalgam, to produce a good summary overview diagram of its parts.
The "before" picture of dependencies, with a big strongly connected component of 25 compilation units:
The picture after the cleanup:
Now no two files outside 3rdparty are in an include dependency cycle, and the folder structure of files is also reorganized. Boxes in the diagram correspond to folders. It is not immediately evident from the picture but the quotient graph of include dependencies by folders is also acyclic.