svn commit: r1888520 - in /subversion/trunk: build.conf subversion/libsvn_subr/task.c
Author: stefan2
Date: Thu Apr 8 14:35:52 2021
New Revision: 1888520
URL: http://svn.apache.org/viewvc?rev=1888520&view=rev
Log:
Initial, single-theaded implementation of the svn_task__t API.
* subversion/libsvn_subr/task.c
(new file): Initial implementation.
Added:
subversion/trunk/subversion/libsvn_subr/task.c (with props)
Modified:
subversion/trunk/build.conf
Modified: subversion/trunk/build.conf
URL:
http://svn.apache.org/viewvc/subversion/trunk/build.conf?rev=1888520&r1=1888519&r2=1888520&view=diff
==
--- subversion/trunk/build.conf (original)
+++ subversion/trunk/build.conf Thu Apr 8 14:35:52 2021
@@ -395,7 +395,7 @@ msvc-export =
private\svn_temp_serializer.h private\svn_io_private.h
private\svn_sorts_private.h private\svn_auth_private.h
private\svn_string_private.h private\svn_magic.h
-private\svn_subr_private.h private\svn_mutex.h
+private\svn_subr_private.h private\svn_mutex.h private\svn_task.h
private\svn_thread_cond.h private\svn_waitable_counter.h
private\svn_packed_data.h private\svn_object_pool.h private\svn_cert.h
private\svn_config_private.h private\svn_dirent_uri_private.h
Added: subversion/trunk/subversion/libsvn_subr/task.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/task.c?rev=1888520&view=auto
==
--- subversion/trunk/subversion/libsvn_subr/task.c (added)
+++ subversion/trunk/subversion/libsvn_subr/task.c Thu Apr 8 14:35:52 2021
@@ -0,0 +1,723 @@
+/* task.c : Implement the parallel task execution machine.
+ *
+ *
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements. See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership. The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *KIND, either express or implied. See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ *
+ */
+
+#include "private/svn_task.h"
+
+#include
+
+#include "private/svn_atomic.h"
+#include "private/svn_thread_cond.h"
+
+/* Top of the task tree.
+ *
+ * It is accessible from all tasks and contains all necessary ressource
+ * pools and synchronization mechanisms.
+ */
+typedef struct root_t
+{
+ /* The actual root task. */
+ svn_task__t *task;
+
+ /* Pools "segregated" for reduced lock contention when multi-threading.
+ * Ordered by lifetime of the objects allocated in them (long to short).
+ */
+
+ /* Allocate tasks and callbacks here.
+ * These have the longest lifetimes and will (currently) not be released
+ * until this root object gets cleaned up.
+ */
+ apr_pool_t *task_pool;
+
+ /* Allocate per-task parameters (process_baton) from sub-pools of this.
+ * They should be cleaned up as soon as the respective task has been
+ * has been processed (the parameters will not be needed anymore).
+ */
+ apr_pool_t *process_pool;
+
+ /* Allocate per-task output_t as well as the actual outputs here.
+ * Allocation will happen just before calling the processing function.
+ * Release the memory immediately afterwards, unless some actual output
+ * has been produced.
+ */
+ apr_pool_t *output_pool;
+
+ /* Context construction parameters as passed in to svn_task__run(). */
+ svn_task__thread_context_constructor_t context_constructor;
+ void *context_baton;
+
+} root_t;
+
+/* Sub-structure of svn_task__t containing that task's output.
+ */
+typedef struct output_t
+{
+ /* (Last part of the) output produced by the task.
+ * If the task has sub-tasks, additional output (produced before creating
+ * the sub-task) may be found in the respective sub-tasks
+ * PRIOR_PARENT_OUTPUT. NULL, if no output was produced.
+ */
+ void *output;
+
+ /* Error code returned by the proccessing function. */
+ svn_error_t *error;
+
+ /* Parent tasks output before this task has been created, i.e. the part
+ * that shall be passed to the output function before this tasks' output.
+ * NULL, if there is no prior parent output.
+ *
+ * This has to be allocated in the parant's OUTPUT->POOL.
+ */
+ void *prior_parent_output;
+
+ /* The tasks' output may be split into multiple parts, produced before
svn commit: r1888523 - in /subversion/trunk/subversion: include/private/svn_task.h libsvn_subr/task.c
Author: stefan2
Date: Thu Apr 8 15:46:02 2021
New Revision: 1888523
URL: http://svn.apache.org/viewvc?rev=1888523&view=rev
Log:
For the svn_task__t API, incorporate feedback on documentation and
use clearer parameter names.
* subversion/include/private/svn_task.h
(): Typos and clarify order of output and error handling.
(svn_task__create_process_pool): minor change in docstring wording.
(svn_task__add,
svn_task__add_similar): Rename TASK to CURRENT for clarity.
* subversion/libsvn_subr/task.c
(svn_task__add,
svn_task__add_similar): Rename TASK to CURRENT for clarity.
Modified:
subversion/trunk/subversion/include/private/svn_task.h
subversion/trunk/subversion/libsvn_subr/task.c
Modified: subversion/trunk/subversion/include/private/svn_task.h
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_task.h?rev=1888523&r1=1888522&r2=1888523&view=diff
==
--- subversion/trunk/subversion/include/private/svn_task.h (original)
+++ subversion/trunk/subversion/include/private/svn_task.h Thu Apr 8 15:46:02
2021
@@ -40,23 +40,30 @@ extern "C" {
*
* During execution, a task may add further sub-tasks - equivalent to
* sub-function calls. They will be executed after their parent task
- * has been processed forming an growing tree of *isolated* tasks.
+ * has been processed forming a growing tree of *isolated* tasks.
*
* Tasks may be executed in arbitrary order, concurrently and in parallel.
* To guarantee consistent output order and error handling, every task
* consists of two functions. The first is the "process function" that
* should perform the bulk of the work, may be executed in some worker
- * thread and may produce some result. The latter is later passed into
- * the second function, the "output function". This one is called in the
- * main thread and strictly in pre-order wrt. the position of the respective
- * task within the tree. Both, process and output functions, may add
- * further sub-tasks as needed.
+ * thread and may produce some result.
*
- * Errors are detected in strictly the same order, with only the first one
+ * The latter is later passed into the second function, the "output function".
+ * This one is called in the main thread and strictly in post-order wrt.
+ * the position of the respective task within the tree. However, a task
+ * may produce output that needs to be processed before any sub-task or
+ * in between them - just like you would in normal function / sub-function
+ * code. Such partial outputs can be handed in whenever a new sub-task
+ * is being added and the output function will be called on them right
+ * before handling the output of the respective sub-task.
+ *
+ * Both, process and output functions, may add further sub-tasks as needed.
+ *
+ * Errors are detected in strictly in post-order, with only the first one
* being returned from the task runner. In particular, it may not be the
* first error that occurs timewise but only the first one encountered when
* walking the tree and checking the processing results for errors. Because
- * it takes some time make the workers cancel all outstanding tasks,
+ * it takes some time to make the workers cancel all outstanding tasks,
* additional errors may occur before and after the one that ultimately
* gets reported. All those errors are being swallowed.
*
@@ -181,15 +188,15 @@ svn_error_t *svn_task__run(
* into either @a svn_task__add() or @a svn_task__add_similar() - even if
* you use a @c NULL process baton.
*
- * @todo Consider replace these pools with a single pool at the parent
- * and turn this into a simple getter. We will end up with a lot fewer
+ * @todo Consider replacing these pools with a single pool at the parent
+ * and turning this into a simple getter. We will end up with a lot fewer
* pools that OTOH may live a lot longer.
*/
apr_pool_t *svn_task__create_process_pool(
svn_task__t *parent);
/**
- * Append a new sub-task to the current @a task with the given
+ * Append a new sub-task to the @a current task with the given
* @a process_pool. The latter must have been allocated with
* @a svn_task__create_process_pool() for @a task.
*
@@ -205,7 +212,7 @@ apr_pool_t *svn_task__create_process_poo
* and / or processing will take place.
*/
svn_error_t *svn_task__add(
- svn_task__t *task,
+ svn_task__t *current,
apr_pool_t *process_pool,
void *partial_output,
svn_task__process_func_t process_func,
@@ -220,7 +227,7 @@ svn_error_t *svn_task__add(
* as for the current task. This is useful for recursive tasks.
*/
svn_error_t *svn_task__add_similar(
- svn_task__t *task,
+ svn_task__t *current,
apr_pool_t *process_pool,
void *partial_output,
void *process_baton);
Modified: subversion/trunk/subversion/libsvn_subr/task.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/task.c?rev=1
Re: svn commit: r1886494 - /subversion/site/staging/docs/community-guide/releasing.part.html
Den tis 16 feb. 2021 kl 16:20 skrev Daniel Sahlberg < daniel.l.sahlb...@gmail.com>: > Den tis 16 feb. 2021 kl 12:44 skrev Daniel Sahlberg > : > >> P.S. While reviewing this I noticed that > >> https://subversion-staging.apache.org/favicon.ico is different to > >> https://svn.apache.org/repos/asf/subversion/site/staging/favicon.ico: > an Apache > >> feather v. a Subversion logo. > > > > > > Hmm. That is very strange. I've pinged infra on Slack. > > Infra says: > > looks like it's an overall override for "generic sites" that they all > have the ASF favicon due to the dash in subversion-staging it gets > overridden > > Ie, it is not a fault in our repository but an httpd.conf thing for the > site. > > I asked to have this removed and was requested to add it to Jira: > https://issues.apache.org/jira/browse/INFRA-21429 It took just short of two months but Infra has changed the override and the staging site is showing the correct favicon, ie the one in the repository. Kind regards Daniel
svn commit: r1888531 - /subversion/trunk/subversion/libsvn_subr/task.c
Author: danielsh Date: Thu Apr 8 19:24:07 2021 New Revision: 1888531 URL: http://svn.apache.org/viewvc?rev=1888531&view=rev Log: * subversion/libsvn_subr/task.c: Add form feed section breaks. Modified: subversion/trunk/subversion/libsvn_subr/task.c Modified: subversion/trunk/subversion/libsvn_subr/task.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/task.c?rev=1888531&r1=1888530&r2=1888531&view=diff == --- subversion/trunk/subversion/libsvn_subr/task.c (original) +++ subversion/trunk/subversion/libsvn_subr/task.c Thu Apr 8 19:24:07 2021 @@ -359,6 +359,7 @@ apr_pool_t *svn_task__create_process_poo return svn_pool_create(parent->root->process_pool); } + /* Removing tasks from the tree */ /* Remove TASK from the parent tree. @@ -401,6 +402,7 @@ static void free_sub_tasks(svn_task__t * task->parent->first_sub = task->next; } + /* Picking the next task to process */ /* Utility function that follows the chain of siblings and returns the first @@ -459,6 +461,7 @@ static void unready_task(svn_task__t *ta } } + /* Task processing and outputting results */ /* The forground output_processed() function will now consider TASK's @@ -617,6 +620,7 @@ static svn_error_t *output_processed( return SVN_NO_ERROR; } + /* Execution models */ /* Run the (root) TASK to completion, including dynamically added sub-tasks. @@ -673,6 +677,7 @@ static svn_error_t *execute_serially( return svn_error_trace(task_err); } + /* Root data structure */ svn_error_t *svn_task__run(
svn commit: r1888532 - /subversion/trunk/subversion/libsvn_subr/task.c
Author: danielsh Date: Thu Apr 8 19:34:49 2021 New Revision: 1888532 URL: http://svn.apache.org/viewvc?rev=1888532&view=rev Log: * subversion/libsvn_subr/task.c: Add one more formfeed. Modified: subversion/trunk/subversion/libsvn_subr/task.c Modified: subversion/trunk/subversion/libsvn_subr/task.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/task.c?rev=1888532&r1=1888531&r2=1888532&view=diff == --- subversion/trunk/subversion/libsvn_subr/task.c (original) +++ subversion/trunk/subversion/libsvn_subr/task.c Thu Apr 8 19:34:49 2021 @@ -197,6 +197,7 @@ struct svn_task__t output_t *output; }; + /* Adding tasks to the tree. */ /* Return the index of the first immediate sub-task of TASK with a ready
svn commit: r1888533 - /subversion/trunk/subversion/libsvn_subr/task.c
Author: danielsh
Date: Thu Apr 8 19:41:44 2021
New Revision: 1888533
URL: http://svn.apache.org/viewvc?rev=1888533&view=rev
Log:
* subversion/libsvn_subr/task.c: Spelling and other cosmetic changes.
Modified:
subversion/trunk/subversion/libsvn_subr/task.c
Modified: subversion/trunk/subversion/libsvn_subr/task.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/task.c?rev=1888533&r1=1888532&r2=1888533&view=diff
==
--- subversion/trunk/subversion/libsvn_subr/task.c (original)
+++ subversion/trunk/subversion/libsvn_subr/task.c Thu Apr 8 19:41:44 2021
@@ -27,9 +27,10 @@
#include "private/svn_atomic.h"
#include "private/svn_thread_cond.h"
+
/* Top of the task tree.
*
- * It is accessible from all tasks and contains all necessary ressource
+ * It is accessible from all tasks and contains all necessary resource
* pools and synchronization mechanisms.
*/
typedef struct root_t
@@ -49,7 +50,7 @@ typedef struct root_t
/* Allocate per-task parameters (process_baton) from sub-pools of this.
* They should be cleaned up as soon as the respective task has been
- * has been processed (the parameters will not be needed anymore).
+ * processed (the parameters will not be needed anymore).
*/
apr_pool_t *process_pool;
@@ -72,7 +73,7 @@ typedef struct output_t
{
/* (Last part of the) output produced by the task.
* If the task has sub-tasks, additional output (produced before creating
- * the sub-task) may be found in the respective sub-tasks
+ * each sub-task) may be found in the respective sub-task's
* PRIOR_PARENT_OUTPUT. NULL, if no output was produced.
*/
void *output;
@@ -80,11 +81,11 @@ typedef struct output_t
/* Error code returned by the proccessing function. */
svn_error_t *error;
- /* Parent tasks output before this task has been created, i.e. the part
- * that shall be passed to the output function before this tasks' output.
+ /* Parent task's output before this task has been created, i.e. the part
+ * that shall be passed to the output function before this task's output.
* NULL, if there is no prior parent output.
*
- * This has to be allocated in the parant's OUTPUT->POOL.
+ * This has to be allocated in the parent's OUTPUT->POOL.
*/
void *prior_parent_output;
@@ -93,7 +94,7 @@ typedef struct output_t
* of those sub-tasks but have been allocated in (their parent's) POOL.
*
* This flag indicates if such partial results exist. POOL must not be
- * destroyed in that case, until all sub-tasks outputs have been handled.
+ * destroyed in that case, until all sub-tasks' outputs have been handled.
*/
svn_boolean_t has_partial_results;
@@ -165,10 +166,10 @@ struct svn_task__t
/* The first task, in pre-order, of this sub-tree whose processing has not
* been started yet. This will be NULL, iff for all tasks in this sub-tree,
- * processing has at least been started. If *this==FIRST_READY, this task
+ * processing has at least been started. If this==FIRST_READY, this task
* itself waits for being proccessed.
*
- * Note that immediate and / or indirect sub-tasks may already have been
+ * Note that immediate and/or indirect sub-tasks may already have been
* processed before this one.
*/
svn_task__t *first_ready;
@@ -476,7 +477,7 @@ static void set_processed(svn_task__t *t
* Pending sub-tasks will be ignored. */
static svn_boolean_t is_processed(const svn_task__t *task)
{
- return task->process_pool == NULL;
+ return (task->process_pool == NULL);
}
/* Process a single TASK within the given THREAD_CONTEXT. It may add
@@ -563,8 +564,8 @@ static svn_error_t *output_processed(
/* We will handle this sub-task in the next iteration but we
* may have to process results produced before or in between
- * sub-tasks. Also note that PRIOR_PARENT_OUTPUT will only be
- * set, of the OUTPUT_FUNC is not NULL. */
+ * sub-tasks. Also note that PRIOR_PARENT_OUTPUT being true
+ * implies that OUTPUT_FUNC is not NULL. */
if (output && output->prior_parent_output)
SVN_ERR(callbacks->output_func(
current->parent, output->prior_parent_output,
Review of task.c as of svn commit: r1888520 - in /subversion/trunk: build.conf subversion/libsvn_subr/task.c
Good morning Stefan,
stef...@apache.org wrote on Thu, Apr 08, 2021 at 14:35:52 -:
> Initial, single-theaded implementation of the svn_task__t API.
I've committed some minor fixes in r1888533. Most of them are
straightforward but please double check that all the apostrophes are
where they should be.
Here's also a review of task.c as it stands now.
Index: subversion/include/private/svn_task.h
===
--- subversion/include/private/svn_task.h (revision 1888527)
+++ subversion/include/private/svn_task.h (working copy)
@@ -117,6 +117,9 @@ typedef svn_error_t *(*svn_task__process_func_t)(
* All data that shall be returned by @a svn_task__run() needs to be
* allocated from @a result_pool.
*
+ * @note The lifetime of @a result is not guaranteed. If it is to be returned
+ * from svn_task__run(), it needs to be copied to @a result_pool.
+ *
* @a cancel_func, @a cancel_baton and @a scratch_pool are the usual things.
*/
typedef svn_error_t *(*svn_task__output_func_t)(
Index: subversion/libsvn_subr/task.c
===
--- subversion/libsvn_subr/task.c (revision 1888533)
+++ subversion/libsvn_subr/task.c (working copy)
@@ -139,7 +139,8 @@ struct svn_task__t
/* Parent task.
* NULL for the root node:
- * (PARENT==NULL) == (*this==ROOT->TASK).
+ * (PARENT==NULL) == (this==ROOT->TASK).
+ * ### assert() that?
*/
svn_task__t *parent;
@@ -177,6 +178,7 @@ struct svn_task__t
/* Task state. */
/* The callbacks to use. Never NULL. */
+ /* ### assert() that? */
callbacks_t *callbacks;
/* Process baton to pass into CALLBACKS->PROCESS_FUNC. */
@@ -186,7 +188,7 @@ struct svn_task__t
* Sub-pool of ROOT->PROCESS_POOL.
*
* NULL, iff processing of this task has completed (sub-tasks may still
- * need processing). Used to check whether processing has been completed.
+ * need processing).
*/
apr_pool_t *process_pool;
@@ -203,12 +205,13 @@ struct svn_task__t
/* Return the index of the first immediate sub-task of TASK with a ready
* sub-task in its respective sub-tree. TASK must have at least one such
- * sub-task.
+ * proper sub-task.
*/
static apr_size_t first_ready_sub_task_idx(const svn_task__t *task)
{
svn_task__t *sub_task = task->first_ready;
assert(sub_task);
+ assert(sub_task != task);
while (sub_task->parent != task)
sub_task = sub_task->parent;
@@ -391,6 +394,8 @@ static svn_error_t *remove_task(svn_task__t *task)
* mainly to prevent error leaks when terminating the task runner early.
* It is better to have the standard pool cleanups to deal with memory
* management in that case.
+ *
+ * ### Also resets FIRST_SUB
*/
static void free_sub_tasks(svn_task__t *task)
{
@@ -503,6 +508,7 @@ static void process(svn_task__t *task,
{
/* Depending on whether there is prior parent output, we may or
* may not have already an OUTPUT structure allocated for TASK. */
+ /* ### Aside: The names could be clearer. As it is, output ==
task->output != output->output */
output_t *output = ensure_output(task);
output->error = callbacks->process_func(&output->output, task,
thread_context,
@@ -555,7 +561,12 @@ static svn_error_t *output_processed(
/* Post-order, i.e. dive into sub-tasks first.
*
* Note that the "background" processing for CURRENT has completed
- * and only the output function may add no further sub-tasks. */
+ * and the "foreground" output function may add further sub-tasks. */
+ /* ### The 'if' branch processes
current->first_sub->output->prior_parent_output —
+ * ### that's output produced by current — in this iteration, and only in
+ * ### the next iteration will handle current->first_sub's subtree's
+ * ### outputs. That's not post-order, is it?
+ */
if (current->first_sub)
{
current = current->first_sub;
HTH,
Daniel
