From cdb1ee21a04fcf587843df651d95b523a7b896a8 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Fri, 22 Jun 2018 11:29:05 -0700 Subject: [PATCH 1/2] Clean-up parent / owner reference of children during clonning Summary: This diff cleans up the parent / owner references for children of ReactShadowNode / YogaNode during cloning. The reason of this behavior is to avoid retaining every generation of trees during cloning. This fixes a memory leak detected when running the ProgressBarExample.android.js in catalyst app Reviewed By: fkgozali Differential Revision: D8019894 fbshipit-source-id: b0d38f0c836ffec534f64fa1adbd7511ecf3473d --- java/com/facebook/yoga/YogaNode.java | 12 +++++++++--- yoga/Yoga.cpp | 12 ++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/java/com/facebook/yoga/YogaNode.java b/java/com/facebook/yoga/YogaNode.java index 90caf8f8..f8b895b1 100644 --- a/java/com/facebook/yoga/YogaNode.java +++ b/java/com/facebook/yoga/YogaNode.java @@ -1,8 +1,9 @@ /* - * Copyright (c) 2014-present, Facebook, Inc. + * Copyright (c) 2014-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. */ package com.facebook.yoga; @@ -185,6 +186,11 @@ public class YogaNode implements Cloneable { clonedYogaNode.mOwner = null; clonedYogaNode.mChildren = mChildren != null ? (List) ((ArrayList) mChildren).clone() : null; + if (clonedYogaNode.mChildren != null) { + for (YogaNode child : clonedYogaNode.mChildren) { + child.mOwner = null; + } + } return clonedYogaNode; } catch (CloneNotSupportedException ex) { // This class implements Cloneable, this should not happen diff --git a/yoga/Yoga.cpp b/yoga/Yoga.cpp index c6a8ffd2..3c83ce08 100644 --- a/yoga/Yoga.cpp +++ b/yoga/Yoga.cpp @@ -1,8 +1,9 @@ -/** - * Copyright (c) 2014-present, Facebook, Inc. +/* + * Copyright (c) 2014-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. */ #include "Yoga.h" @@ -243,6 +244,9 @@ YGNodeRef YGNodeClone(YGNodeRef oldNode) { oldNode->getConfig(), node != nullptr, "Could not allocate memory for node"); + for (auto &item : oldNode->getChildren()) { + item->setOwner(nullptr); + } gNodeInstanceCount++; node->setOwner(nullptr); return node; From a67c55532076e64f78d1897c39e0f91e709dbc1e Mon Sep 17 00:00:00 2001 From: Taras Tsugrii Date: Sat, 23 Jun 2018 18:23:21 -0700 Subject: [PATCH 2/2] Fix deprecated glob usage. Summary: https://our.intern.facebook.com/intern/wiki/Buck/python-to-skylark/ Differential Revision: D8595731 fbshipit-source-id: 0e3046a7fd2a25e9b13462713ae9a008ad546770 --- BUCK | 3 +- YogaKit/BUCK | 3 +- benchmark/BUCK | 3 +- lib/fb/BUCK | 3 +- lib/gtest/BUCK | 3 +- yoga_defs.bzl | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 86 insertions(+), 10 deletions(-) diff --git a/BUCK b/BUCK index fd87a83b..4b809f69 100644 --- a/BUCK +++ b/BUCK @@ -2,8 +2,7 @@ # # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. - -load("//:yoga_defs.bzl", "BASE_COMPILER_FLAGS", "GTEST_TARGET", "LIBRARY_COMPILER_FLAGS", "yoga_cxx_library", "yoga_cxx_test", "yoga_dep") +load("//:yoga_defs.bzl", "BASE_COMPILER_FLAGS", "GTEST_TARGET", "LIBRARY_COMPILER_FLAGS", "subdir_glob", "yoga_cxx_library", "yoga_cxx_test", "yoga_dep") GMOCK_OVERRIDE_FLAGS = [ # gmock does not mark mocked methods as override, ignore the warnings in tests diff --git a/YogaKit/BUCK b/YogaKit/BUCK index 14198edb..9675d049 100644 --- a/YogaKit/BUCK +++ b/YogaKit/BUCK @@ -2,8 +2,7 @@ # # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. - -load("//:yoga_defs.bzl", "yoga_apple_library", "yoga_apple_test", "yoga_dep") +load("//:yoga_defs.bzl", "subdir_glob", "yoga_apple_library", "yoga_apple_test", "yoga_dep") COMPILER_FLAGS = [ "-fobjc-arc", diff --git a/benchmark/BUCK b/benchmark/BUCK index 2b30af78..82864373 100644 --- a/benchmark/BUCK +++ b/benchmark/BUCK @@ -2,8 +2,7 @@ # # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. - -load("//:yoga_defs.bzl", "yoga_cxx_binary", "yoga_dep") +load("//:yoga_defs.bzl", "subdir_glob", "yoga_cxx_binary", "yoga_dep") yoga_cxx_binary( name = "benchmark", diff --git a/lib/fb/BUCK b/lib/fb/BUCK index e72c3cb6..cc2502ec 100644 --- a/lib/fb/BUCK +++ b/lib/fb/BUCK @@ -2,8 +2,7 @@ # # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. - -load("//:yoga_defs.bzl", "ANDROID", "FBJNI_JAVA_TARGET", "JNI_TARGET", "YOGA_ROOTS", "yoga_cxx_library", "yoga_prebuilt_cxx_library") +load("//:yoga_defs.bzl", "ANDROID", "FBJNI_JAVA_TARGET", "JNI_TARGET", "YOGA_ROOTS", "subdir_glob", "yoga_cxx_library", "yoga_prebuilt_cxx_library") yoga_prebuilt_cxx_library( name = "ndklog", diff --git a/lib/gtest/BUCK b/lib/gtest/BUCK index 167d70eb..e489c69c 100644 --- a/lib/gtest/BUCK +++ b/lib/gtest/BUCK @@ -2,8 +2,7 @@ # # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. - -load("//:yoga_defs.bzl", "YOGA_ROOTS", "yoga_cxx_library") +load("//:yoga_defs.bzl", "YOGA_ROOTS", "subdir_glob", "yoga_cxx_library") COMPILER_FLAGS = [ "-std=c++11", diff --git a/yoga_defs.bzl b/yoga_defs.bzl index c6d7120a..9f33a825 100644 --- a/yoga_defs.bzl +++ b/yoga_defs.bzl @@ -59,6 +59,87 @@ LIBRARY_COMPILER_FLAGS = BASE_COMPILER_FLAGS + [ "-fPIC", ] +def _paths_join(path, *others): + """Joins one or more path components.""" + result = path + + for p in others: + if p.startswith("/"): # absolute + result = p + elif not result or result.endswith("/"): + result += p + else: + result += "/" + p + + return result + +def subdir_glob(glob_specs, exclude = None, prefix = ""): + """Returns a dict of sub-directory relative paths to full paths. + + The subdir_glob() function is useful for defining header maps for C/C++ + libraries which should be relative the given sub-directory. + Given a list of tuples, the form of (relative-sub-directory, glob-pattern), + it returns a dict of sub-directory relative paths to full paths. + + Please refer to native.glob() for explanations and examples of the pattern. + + Args: + glob_specs: The array of tuples in form of + (relative-sub-directory, glob-pattern inside relative-sub-directory). + type: List[Tuple[str, str]] + exclude: A list of patterns to identify files that should be removed + from the set specified by the first argument. Defaults to []. + type: Optional[List[str]] + prefix: If is not None, prepends it to each key in the dictionary. + Defaults to None. + type: Optional[str] + + Returns: + A dict of sub-directory relative paths to full paths. + """ + if exclude == None: + exclude = [] + + results = [] + + for dirpath, glob_pattern in glob_specs: + results.append( + _single_subdir_glob(dirpath, glob_pattern, exclude, prefix), + ) + + return _merge_maps(*results) + +def _merge_maps(*file_maps): + result = {} + for file_map in file_maps: + for key in file_map: + if key in result and result[key] != file_map[key]: + fail( + "Conflicting files in file search paths. " + + "\"%s\" maps to both \"%s\" and \"%s\"." % + (key, result[key], file_map[key]), + ) + + result[key] = file_map[key] + + return result + +def _single_subdir_glob(dirpath, glob_pattern, exclude = None, prefix = None): + if exclude == None: + exclude = [] + results = {} + files = native.glob([_paths_join(dirpath, glob_pattern)], exclude = exclude) + for f in files: + if dirpath: + key = f[len(dirpath) + 1:] + else: + key = f + if prefix: + key = _paths_join(prefix, key) + results[key] = f + + return results + def yoga_dep(dep): return "//" + dep