You are viewing a plain text version of this content. The canonical link for it is here.
Posted to yarn-issues@hadoop.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2022/10/05 20:06:00 UTC
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17613191#comment-17613191 ]
ASF GitHub Bot commented on YARN-11216:
---------------------------------------
K0K0V0K commented on code in PR #4655:
URL: https://github.com/apache/hadoop/pull/4655#discussion_r985825269
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/ConfigurationProperties.java:
##########
@@ -55,6 +59,17 @@ public ConfigurationProperties(Map<String, String> props) {
storePropertiesInPrefixNodes(props);
}
+ /**
+ * A constructor defined in order to conform to the type used by
+ * {@code Configuration}. It must only be called by String keys and values.
+ * @param props properties to store
+ * @param whiteListPrefix only those properties will be in the nodes
+ * which starts with one of the provided prefixes.
+ */
+ public ConfigurationProperties(Map<String, String> props, String... whiteListPrefix) {
+ this(Maps.filterKeys(props, key -> StringUtils.startsWithAny(key, whiteListPrefix)));
Review Comment:
Thanks for the review @9uapaw and sorry for the late response
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/ConfigurationProperties.java:
##########
@@ -55,6 +59,17 @@ public ConfigurationProperties(Map<String, String> props) {
storePropertiesInPrefixNodes(props);
}
+ /**
+ * A constructor defined in order to conform to the type used by
+ * {@code Configuration}. It must only be called by String keys and values.
+ * @param props properties to store
+ * @param whiteListPrefix only those properties will be in the nodes
+ * which starts with one of the provided prefixes.
+ */
+ public ConfigurationProperties(Map<String, String> props, String... whiteListPrefix) {
+ this(Maps.filterKeys(props, key -> StringUtils.startsWithAny(key, whiteListPrefix)));
Review Comment:
Thanks for the review @9uapaw and sorry for the late response
I dont think the lambda has performance issue here. I created a small perf test to prove it, and based on it the vanilia solution is way slower than guava. I suggest the Hashmap.put() method requires some mem allocation time when the map reach the limit.
[benchmark_code.txt](https://github.com/apache/hadoop/files/9698166/benchmark_code.txt)
[benchmark_log.txt](https://github.com/apache/hadoop/files/9698167/benchmark_log.txt)
> Avoid unnecessary reconstruction of ConfigurationProperties
> -----------------------------------------------------------
>
> Key: YARN-11216
> URL: https://issues.apache.org/jira/browse/YARN-11216
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: capacity scheduler
> Reporter: András Győri
> Assignee: Bence Kosztolnik
> Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> ConfigurationProperties is expensive to create, however, due to its immutable nature it is possible to copy it/share it between configuration objects (eg. create a copy constructor).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org