alibaba / GraphScope

🔨 🍇 💻 🚀 GraphScope: A One-Stop Large-Scale Graph Computing System from Alibaba | 一站式图计算系统
https://graphscope.io
Apache License 2.0
3.3k stars 446 forks source link

Remaining improvement of headers cleanup for analytical engine #1582

Open acezen opened 2 years ago

acezen commented 2 years ago

Is your feature request related to a problem? Please describe.

Describe the solution you'd like A clear and concise description of what you want to happen.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context

1541

yecol commented 1 year ago

Using tools to analysis what headers to include. e.g., https://include-what-you-use.org/

dashanji commented 1 year ago

I have tested a lot, and it doesn't make sense to only delete the unused headers as the headers depend on each other. A possible way to refactor the headers to achieve minimal dependencies, I'll dig into the code and present a proposal for how to split the headers.

dashanji commented 1 year ago

The test steps:

Step 1: Use the include-what-you-use to find which header is used, and the part result is as follows.

/opt/graphscope/include/graphscope/frame/property_graph_frame.cc should remove these lines:
- #include "vineyard/common/util/macros.h"  // lines 19-19

The full include-list for /opt/graphscope/include/graphscope/frame/property_graph_frame.cc:
#include <basic/ds/hashmap.vineyard.h>                    // for operator!=
#include <client/ds/object_factory.h>                     // for static_poin...
#include <common/util/json.h>                             // for json
#include <common/util/status.h>                           // for Status, VIN...
#include <common/util/typename.h>                         // for type_name
#include <common/util/uuid.h>                             // for ObjectID
#include <glog/logging.h>                                 // for LogMessage
#include <google/protobuf/any.pb.h>                       // for Any
#include <grape/worker/comm_spec.h>                       // for CommSpec
#include <graph/fragment/arrow_fragment.vineyard.h>       // for ArrowFragment
#include <graph/fragment/arrow_fragment_group.h>          // for ArrowFragme...
#include <graph/fragment/property_graph_types.h>          // for AdjList
#include <graph/utils/context_protocols.h>                // for TypeToInt
#include <graph/vertex_map/arrow_vertex_map.h>            // for ArrowVertexMap
#include <mpi.h>                                          // for MPI_Barrier
#include <stdint.h>                                       // for uint64_t
#include <boost/leaf/detail/mp11.hpp>                     // for leaf
#include <boost/leaf/error.hpp>                           // for BOOST_LEAF_...
#include <boost/leaf/result.hpp>                          // for result
#include <map>                                            // for map
#include <memory>                                         // for shared_ptr
#include <sstream>                                        // for stringstream
#include <string>                                         // for string, ope...
#include <type_traits>                                    // for is_same
#include <unordered_map>                                  // for unordered_map
#include <utility>                                        // for move, pair
#include <vector>                                         // for vector
#include "core/error.h"                                   // for ErrorCode
#include "core/fragment/dynamic_fragment.h"               // for DynamicFrag...
#include "core/io/property_parser.h"                      // for ParseCreate...
#include "core/loader/arrow_fragment_loader.h"            // for arrow_fragm...
#include "core/loader/arrow_to_dynamic_converter.h"       // for ArrowToDyna...
#include "core/loader/dynamic_to_arrow_converter.h"       // for DynamicToAr...
#include "core/object/dynamic.h"                          // for Stringify
#include "core/object/fragment_wrapper.h"                 // for FragmentWra...
#include "core/object/i_fragment_wrapper.h"               // for IFragmentWr...
#include "core/server/rpc_utils.h"                        // for GSParams
#include "core/utils/transform_utils.h"                   // for TransformUtils
#include "core/vertex_map/arrow_projected_vertex_map.h"   // for ArrowProjec...
#include "graphscope/proto/graph_def.pb.h"                // for GraphDefPb
#include "graphscope/proto/types.pb.h"                    // for ParamKey
#include "vineyard/client/client.h"                       // for Client
#include "vineyard/graph/loader/fragment_loader_utils.h"  // for ConstructFr...
#include "vineyard/graph/loader/gar_fragment_loader.h"    // for gar_fragmen...
#include "vineyard/graph/writer/arrow_fragment_writer.h"  // for ArrowFragme...
---

Step 2: then delete the unused headers in the property_graph_frame.cc.

//#include <memory>

#include "vineyard/client/client.h"
#include "vineyard/common/util/macros.h"
//#include "vineyard/graph/fragment/arrow_fragment.h"
#include "vineyard/graph/loader/fragment_loader_utils.h"

#include "vineyard/graph/loader/gar_fragment_loader.h"
#include "vineyard/graph/writer/arrow_fragment_writer.h"

//#include "core/config.h"
#include "core/error.h"
#include "core/fragment/dynamic_fragment.h"
//#include "core/io/property_parser.h"
#include "core/loader/arrow_fragment_loader.h"
#include "core/loader/arrow_to_dynamic_converter.h"
#include "core/loader/dynamic_to_arrow_converter.h"
#include "core/object/fragment_wrapper.h"
#include "core/server/rpc_utils.h"
//#include "core/utils/fragment_traits.h"
//#include "core/vertex_map/arrow_projected_vertex_map.h"
#include "graphscope/proto/attr_value.pb.h"
#include "graphscope/proto/graph_def.pb.h"

Step 3: Build a graph and add the -H to see what headers are complied. Actually, these headers are still included in other headers. E.g, for the header vineyard/graph/fragment/arrow_fragment.h.

. /opt/vineyard/include/vineyard/graph/loader/fragment_loader_utils.h
.. /opt/vineyard/include/vineyard/graph/loader/basic_ev_fragment_loader.h
... /opt/vineyard/include/vineyard/graph/fragment/arrow_fragment.h

It shows the fragment_loader_utils.h include the arrow_fragment.h so we can't only delete the headers at tall. It's better to split the headers by features.