Address

velox/common/dynamic_registry/CMakeLists.txt

@@ -12,9 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-velox_add_library(velox_dynamic_function_loader DynamicLibraryLoader.cpp)
+velox_add_library(velox_dynamic_library_loader DynamicLibraryLoader.cpp)
 
-velox_link_libraries(velox_dynamic_function_loader PRIVATE velox_exception)
+velox_link_libraries(velox_dynamic_library_loader PRIVATE velox_exception)
 
 if(${VELOX_BUILD_TESTING})
   add_subdirectory(tests)

velox/common/dynamic_registry/DynamicLibraryLoader.h

@@ -32,10 +32,12 @@ namespace facebook::velox {
 /// The function uses dlopen to load the shared library.
 /// It then searches for the "void registry()" C function which typically
 /// contains all the registration code for the UDFs defined in library. After
-/// locating the function it executes the registration bringing the UDFs in the
-/// scope of the Velox runtime.
+/// locating the function it executes the registration bringing the
+/// user-defined Velox components such as function in the scope of the
+/// Velox runtime.
 ///
-/// If the same library is loaded twice then a no-op scenerio will happen.
+/// If the same library is loaded twice then a no-op scenerio will happen as the
+/// function will be overwritten by itself.
 
 void loadDynamicLibrary(const char* fileName);
 

velox/common/dynamic_registry/DynamicUdf.h

@@ -22,6 +22,8 @@
 #include "velox/functions/Udf.h"
 
 extern "C" {
+// The "registry()" declaration and "check()" function ensure the correct 
+// registry signature function is defined by the user.
 void registry();
 void check() {
   registry();

velox/common/dynamic_registry/README.md

@@ -13,20 +13,14 @@ Make sure to also include the necessary header file:
 2. Register functions dynamically by creating .dylib (MacOS) or .so (Linux) shared libraries.
 These shared libraries may be made using CMakeLists like the following:
 
-* Linux
-
-    ```
-    add_library(name_of_dynamic_fn SHARED TestFunction.cpp)
-    target_link_libraries(name_of_dynamic_fn PRIVATE xsimd fmt::fmt)
     ```
-    Above, the xsimd and fmt::fmt libraries are required for all necessary symbols to be defined when loading the TestFunction.cpp dynamically
-* MacOS:
+        add_library(name_of_dynamic_fn SHARED TestFunction.cpp)
+        target_link_libraries(name_of_dynamic_fn PRIVATE fmt::fmt glog::glog xsimd)
+        target_link_options(name_of_dynamic_fn PRIVATE "-Wl,-undefined,dynamic_lookup")
     ```
-    add_library(name_of_dynamic_fn SHARED TestFunction.cpp)
-    target_link_libraries(name_of_dynamic_fn PRIVATE Folly::folly xsimd)
-    ``` 
+    Above, the `fmt::fmt` and `xsimd` libraries are required for all necessary symbols 
+    to be defined when loading the `TestFunction.cpp` dynamically. 
+    Additionally `glog::glog` is currently required on MacOs. 
+    The `target_link_options` allows for symbols to be resolved at runtime on MacOS. 
 
-    Additionally, the below flag allows symbols to be resolved at runtime:
-    ```
-    target_link_options(name_of_dynamic_fn PRIVATE "-Wl,-undefined,dynamic_lookup")
-    ```
+    On Linux `glog::glog` and the `target_link_options` are optional.