Skip to content

Commit

Permalink
Feedback updates
Browse files Browse the repository at this point in the history
  • Loading branch information
@drobison00
drobison00 committed Oct 19, 2022
1 parent 97616d2 commit 6e342d4
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 38 deletions.
14 changes: 7 additions & 7 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ include(cmake/setup_iwyu.cmake)
add_library(libsrf
src/internal/data_plane/callbacks.cpp
src/internal/data_plane/client.cpp
src/internal/data_plane/resources.cpp
src/internal/data_plane/request.cpp
src/internal/data_plane/resources.cpp
src/internal/data_plane/server.cpp
src/internal/executor/executor.cpp
src/internal/executor/iexecutor.cpp
Expand All @@ -166,10 +166,10 @@ add_library(libsrf
src/internal/pipeline/port_graph.cpp
src/internal/pipeline/resources.cpp
src/internal/resources/manager.cpp
src/internal/resources/partition_resources.cpp
src/internal/resources/partition_resources_base.cpp
src/internal/runnable/engine_factory.cpp
src/internal/resources/partition_resources.cpp
src/internal/runnable/engine.cpp
src/internal/runnable/engine_factory.cpp
src/internal/runnable/engines.cpp
src/internal/runnable/fiber_engine.cpp
src/internal/runnable/fiber_engines.cpp
Expand Down Expand Up @@ -200,8 +200,8 @@ add_library(libsrf
src/internal/system/system.cpp
src/internal/system/system_provider.cpp
src/internal/system/thread.cpp
src/internal/system/thread_pool.cpp
src/internal/system/thread.cpp
src/internal/system/thread_pool.cpp
src/internal/system/topology.cpp
src/internal/ucx/context.cpp
src/internal/ucx/endpoint.cpp
Expand All @@ -214,6 +214,8 @@ add_library(libsrf
src/internal/utils/parse_config.cpp
src/internal/utils/parse_ints.cpp
src/internal/utils/shared_resource_bit_map.cpp
src/public/benchmarking/tracer.cpp
src/public/benchmarking/trace_statistics.cpp
src/public/benchmarking/util.cpp
src/public/channel/channel.cpp
src/public/codable/encoded_object.cpp
Expand All @@ -224,6 +226,7 @@ add_library(libsrf
src/public/core/logging.cpp
src/public/cuda/device_guard.cpp
src/public/cuda/sync.cpp
src/public/experimental/modules/segment_modules.cpp
src/public/manifold/manifold.cpp
src/public/memory/buffer_view.cpp
src/public/metrics/counter.cpp
Expand All @@ -239,16 +242,13 @@ add_library(libsrf
src/public/options/resources.cpp
src/public/options/services.cpp
src/public/options/topology.cpp
src/public/experimental/modules/segment_modules.cpp
src/public/runnable/context.cpp
src/public/runnable/launcher.cpp
src/public/runnable/runnable.cpp
src/public/runnable/runner.cpp
src/public/runnable/types.cpp
src/public/segment/builder.cpp
src/public/segment/definition.cpp
src/public/benchmarking/trace_statistics.cpp
src/public/benchmarking/tracer.cpp
src/public/utils/bytes_to_string.cpp
src/public/utils/thread_utils.cpp
src/public/utils/type_utils.cpp
Expand Down
18 changes: 8 additions & 10 deletions include/srf/experimental/modules/segment_modules.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
* limitations under the License.
*/

#pragma once

#include "nlohmann/json.hpp"

#include "srf/segment/object.hpp"
Expand All @@ -25,8 +27,6 @@
#include <utility>
#include <vector>

#pragma once

namespace srf::segment {
class Builder;
}
Expand All @@ -36,8 +36,6 @@ namespace srf::modules {
class SegmentModule
{
public:
friend segment::Builder;

using segment_module_port_map_t = std::map<std::string, std::shared_ptr<segment::ObjectProperties>>;
using segment_module_port_t = std::shared_ptr<segment::ObjectProperties>;
using segment_module_typeinfo_map_t = std::map<std::string, const std::type_info*>;
Expand All @@ -56,19 +54,19 @@ class SegmentModule
* Return vector of input names -- these are only understood by the SegmentModule
* @return std::vector
*/
const std::vector<std::string> input_ids() const;
const std::vector<std::string>& input_ids() const;

/**
* Return a vector of output names -- these are only understood by the SegmentModule class
* @return std::vector
*/
const std::vector<std::string> output_ids() const;
const std::vector<std::string>& output_ids() const;

/**
* Return a set of ObjectProperties for module input_ids
* @return ObjectProperties
*/
const segment_module_port_map_t input_ports() const;
const segment_module_port_map_t& input_ports() const;

/**
* Return the ObjectProperties object corresponding to input_name
Expand All @@ -81,7 +79,7 @@ class SegmentModule
* Return a map of module port id : type indices
* @return std::map
*/
const std::map<std::string, const std::type_info*> input_port_type_ids() const;
const std::map<std::string, const std::type_info*>& input_port_type_ids() const;

/**
* Return the type index of a given input name
Expand All @@ -106,7 +104,7 @@ class SegmentModule
* Return a map of module port id : type indices
* @return std::map
*/
const segment_module_typeinfo_map_t output_port_type_ids() const;
const segment_module_typeinfo_map_t& output_port_type_ids() const;

/**
* Return the type index of a given input name
Expand Down Expand Up @@ -170,4 +168,4 @@ class SegmentModule
const nlohmann::json m_config;
};

} // namespace srf::modules
} // namespace srf::modules
2 changes: 1 addition & 1 deletion include/srf/segment/builder.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class Builder final

module.initialize(*this);

return module;
return std::move(module);
}

template <typename SourceNodeTypeT, typename SinkNodeTypeT>
Expand Down
12 changes: 0 additions & 12 deletions python/srf/_pysrf/src/pipeline.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,6 @@ PipelineIngressInfo collect_ingress_info(py::list ids)

bool builder_exists = (cpptype != nullptr && PortRegistry::has_port_util(*cpptype));
std::type_index type_index = builder_exists ? *cpptype : typeid(PyHolder);
if (builder_exists)
{
VLOG(2) << "Ingress builder exists";
} else {
VLOG(2) << "Ingress builder does not exist, defaulting to PyHolder";
}

auto port_util = PortRegistry::find_port_util(type_index);
auto builder_fn = flag_sp_variant ? std::get<1>(port_util->m_ingress_builders)
Expand Down Expand Up @@ -152,12 +146,6 @@ PipelineEgressInfo collect_egress_info(py::list ids)

bool builder_exists = (cpptype != nullptr && PortRegistry::has_port_util(*cpptype));
std::type_index type_index = builder_exists ? *cpptype : typeid(PyHolder);
if (builder_exists)
{
VLOG(2) << "Egress builder exists";
} else {
VLOG(2) << "Egress builder does not exist, defaulting to PyHolder";
}

auto port_util = PortRegistry::find_port_util(type_index);
auto builder_fn =
Expand Down
12 changes: 6 additions & 6 deletions src/public/experimental/modules/segment_modules.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ std::string SegmentModule::get_module_component_name(const std::string& componen
return component_prefix() + component_name;
}

const std::vector<std::string> SegmentModule::input_ids() const
const std::vector<std::string>& SegmentModule::input_ids() const
{
return m_input_port_ids;
}

const SegmentModule::segment_module_port_map_t SegmentModule::input_ports() const
const SegmentModule::segment_module_port_map_t& SegmentModule::input_ports() const
{
return m_input_ports;
}
Expand All @@ -76,7 +76,7 @@ SegmentModule::segment_module_port_t SegmentModule::input_port(const std::string
throw std::invalid_argument(sstream.str());
}

const std::map<std::string, const std::type_info*> SegmentModule::input_port_type_ids() const
const std::map<std::string, const std::type_info*>& SegmentModule::input_port_type_ids() const
{
return m_input_port_type_indices;
}
Expand Down Expand Up @@ -114,7 +114,7 @@ SegmentModule::segment_module_port_t SegmentModule::output_port(const std::strin
throw std::invalid_argument(sstream.str());
}

const SegmentModule::segment_module_typeinfo_map_t SegmentModule::output_port_type_ids() const
const SegmentModule::segment_module_typeinfo_map_t& SegmentModule::output_port_type_ids() const
{
return m_output_port_type_indices;
}
Expand All @@ -133,7 +133,7 @@ const std::type_info* SegmentModule::output_port_type_id(const std::string& outp
throw std::invalid_argument(sstream.str());
}

const std::vector<std::string> SegmentModule::output_ids() const
const std::vector<std::string>& SegmentModule::output_ids() const
{
return m_output_port_ids;
}
Expand Down Expand Up @@ -161,4 +161,4 @@ void SegmentModule::register_output_port(std::string output_name,
m_output_port_type_indices[output_name] = tinfo;
}

} // namespace srf::modules
} // namespace srf::modules
2 changes: 1 addition & 1 deletion tests/test_modules.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,4 +317,4 @@ TEST_F(SegmentTests, ModuleNestingTest)
executor.join();

EXPECT_EQ(packet_count, 4);
}
}
2 changes: 1 addition & 1 deletion tests/test_modules.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,4 +272,4 @@ void NestedModule::initialize(segment::Builder& builder)
configurable_mod.output_port_type_id("configurable_output_x"));
}

} // namespace srf::modules
} // namespace srf::modules

0 comments on commit 6e342d4

Please sign in to comment.