Re: svn commit: r1897460 - in /httpd/httpd/trunk: include/ modules/core/ modules/http2/ modules/ssl/ server/ server/mpm/event/ server/mpm/prefork/ server/mpm/winnt/ server/mpm/worker/

2022-07-10 Thread Yann Ylavic
On Fri, Jul 8, 2022 at 7:08 PM Ivan Zhakov  wrote:
>
> On Tue, 25 Jan 2022 at 20:34,  wrote:
> >
> > -child_handles[i] = CreateThread(NULL, ap_thread_stacksize,
> > -worker_main, (void *) i,
> > -stack_res_flag, );
> > -if (child_handles[i] == 0) {
> > -ap_log_error(APLOG_MARK, APLOG_CRIT, apr_get_os_error(),
> > - ap_server_conf, APLOGNO(00355)
> > - "Child: CreateThread failed. Unable to "
> > +workers[i].num = i;
> > +workers[i].handle = CreateEvent(NULL, TRUE, FALSE, NULL);
> > +if (!workers[i].handle) {
> > +rv = apr_get_os_error();
> > +}
> > +else {
> > +apr_threadattr_t *thread_attr = NULL;
> > +apr_threadattr_create(_attr, pchild);
> > +if (ap_thread_stacksize != 0) {
> > +apr_threadattr_stacksize_set(thread_attr,
> > + ap_thread_stacksize);
> > +}
> > +rv = ap_thread_create([i].thd, thread_attr,
> > +  worker_thread, [i], pchild);
>
> This is performance regression: before this change stack memory was
> 'reserved'. See stack_res_flag in CreateThread call. Now stack memory
> is committed.

So STACK_SIZE_PARAM_IS_A_RESERVATION needs to go to apr_thread_create()?

> >
> >  while (threads_created)
> >  {
> > -HANDLE handle = child_handles[threads_created - 1];
> > +struct worker_info *info = workers[threads_created - 1];
>
> This code doesn't compile:
> [[[
> server\mpm\winnt\child.c(1210,1): error C2440: 'initializing': cannot
> convert from 'worker_info' to 'worker_info *'
> ]]]

r1902636, hopefully.


Regards;
Yann.


Re: svn commit: r1897460 - in /httpd/httpd/trunk: include/ modules/core/ modules/http2/ modules/ssl/ server/ server/mpm/event/ server/mpm/prefork/ server/mpm/winnt/ server/mpm/worker/

2022-07-08 Thread Ivan Zhakov via dev
On Tue, 25 Jan 2022 at 20:34,  wrote:
>
> Author: ylavic
> Date: Tue Jan 25 17:34:57 2022
> New Revision: 1897460
>
> URL: http://svn.apache.org/viewvc?rev=1897460=rev
> Log:
> core: Efficient ap_thread_current() when apr_thread_local() is missing.
>
> #define ap_thread_create, ap_thread_current_create and ap_thread_current to
> their apr-1.8+ equivalent if available, or implement them using the compiler's
> thread_local mechanism if available, or finally provide stubs otherwise.
>
> #define AP_HAS_THREAD_LOCAL to 1 in the two former case or 0 otherwise, while
> AP_THREAD_LOCAL is defined to the compiler's keyword iff AP_HAS_THREAD_LOCAL.
>
> Replace all apr_thread_create() calls with ap_thread_create() so that httpd
> threads can use ap_thread_current()'s pool data as Thread Local Storage.
>
> Bump MMN minor.
>
> * include/httpd.h():
>   Define AP_HAS_THREAD_LOCAL, AP_THREAD_LOCAL (eventually), 
> ap_thread_create(),
>   ap_thread_current_create() and ap_thread_current().
>
> * server/util.c:
>   Implement ap_thread_create(), ap_thread_current_create() and
>   ap_thread_current() when APR < 1.8.
>
> * modules/core/mod_watchdog.c, modules/http2/h2_workers.c,
> modules/ssl/mod_ssl_ct.c:
>   Use ap_thread_create() instead of apr_thread_create.
>
> * server/main.c:
>   Use AP_HAS_THREAD_LOCAL and ap_thread_current_create instead of APR's.
>
> * server/util_pcre.c:
>   Use AP_HAS_THREAD_LOCAL and ap_thread_current instead of APR's.
>
> * server/mpm/event/event.c, server/mpm/worker/worker.c,
> server/mpm/prefork/prefork.c:
>   Use ap_thread_create() instead of apr_thread_create.
>   Create an apr_thread_t/ap_thread_current() for the main chaild thread usable
>   at child_init().
>
> * server/mpm/winnt/child.c:
>   Use ap_thread_create() instead of CreateThread().
>   Create an apr_thread_t/ap_thread_current() for the main chaild thread usable
>
>
> Modified:
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/httpd.h
> httpd/httpd/trunk/modules/core/mod_watchdog.c
> httpd/httpd/trunk/modules/http2/h2_workers.c
> httpd/httpd/trunk/modules/ssl/mod_ssl_ct.c
> httpd/httpd/trunk/server/main.c
> httpd/httpd/trunk/server/mpm/event/event.c
> httpd/httpd/trunk/server/mpm/prefork/prefork.c
> httpd/httpd/trunk/server/mpm/winnt/child.c
> httpd/httpd/trunk/server/mpm/worker/worker.c
> httpd/httpd/trunk/server/util.c
> httpd/httpd/trunk/server/util_pcre.c
>
[..]

> Modified: httpd/httpd/trunk/server/mpm/winnt/child.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/winnt/child.c?rev=1897460=1897459=1897460=diff
> ==
> --- httpd/httpd/trunk/server/mpm/winnt/child.c (original)
> +++ httpd/httpd/trunk/server/mpm/winnt/child.c Tue Jan 25 17:34:57 2022
> @@ -778,24 +778,27 @@ static winnt_conn_ctx_t *winnt_get_conne
>  return context;
>  }
>
> +struct worker_info {
> +apr_thread_t *thd;
> +HANDLE handle;
> +int num;
> +};
> +
>  /*
> - * worker_main()
> + * worker_thread()
>   * Main entry point for the worker threads. Worker threads block in
>   * win*_get_connection() awaiting a connection to service.
>   */
> -static DWORD __stdcall worker_main(void *thread_num_val)
> +static void *APR_THREAD_FUNC worker_thread(apr_thread_t *thd, void *ctx)
>  {
> -apr_thread_t *thd;
> -apr_os_thread_t osthd;
> +struct worker_info *info = ctx;
>  static int requests_this_child = 0;
>  winnt_conn_ctx_t *context = NULL;
> -int thread_num = (int)thread_num_val;
> +int thread_num = info->num;
>  ap_sb_handle_t *sbh;
>  conn_rec *c;
>  apr_int32_t disconnected;
>
> -osthd = apr_os_thread_current();
> -
>  while (1) {
>
>  ap_update_child_status_from_indexes(0, thread_num, SERVER_READY, 
> NULL);
> @@ -827,8 +830,6 @@ static DWORD __stdcall worker_main(void
>  continue;
>  }
>
> -thd = NULL;
> -apr_os_thread_put(, , context->ptrans);
>  c->current_thread = thd;
>
>  ap_process_connection(c, context->sock);
> @@ -843,6 +844,7 @@ static DWORD __stdcall worker_main(void
>
>  ap_update_child_status_from_indexes(0, thread_num, SERVER_DEAD, NULL);
>
> +SetEvent(info->handle);
>  return 0;
>  }
>
> @@ -889,13 +891,21 @@ static void create_listener_thread(void)
>  }
>
>
> +#if AP_HAS_THREAD_LOCAL
> +static apr_status_t main_thread_cleanup(void *arg)
> +{
> +apr_thread_t *thd = arg;
> +apr_pool_destroy(apr_thread_pool_get(thd));
> +return APR_SUCCESS;
> +}
> +#endif
> +
>  void child_main(apr_pool_t *pconf, DWORD parent_pid)
>  {
>  apr_status_t status;
> -apr_hash_t *ht;
>  ap_listen_rec *lr;
>  HANDLE child_events[3];
> -HANDLE *child_handles;
> +struct worker_info *workers;
>  int listener_started = 0;
>  int threads_created = 0;
>  int time_remains;
> @@ -911,8 +921,29 @@ void child_main(apr_pool_t *pconf, DWORD

Re: svn commit: r1897460 - in /httpd/httpd/trunk: include/ modules/core/ modules/http2/ modules/ssl/ server/ server/mpm/event/ server/mpm/prefork/ server/mpm/winnt/ server/mpm/worker/

2022-01-27 Thread Yann Ylavic
On Thu, Jan 27, 2022 at 9:36 AM Ruediger Pluem  wrote:
>
> Would it make sense to move the above code to mpm_common.c as it looks very 
> similar for each MPM?

Good point, in r1897543 I replaced ap_thread_current_create() by
ap_thread_main_create() which is how it's used in httpd anyway.
That's still in util.c because it's used by main() too, one more common code ;)


Regards;
Yann.


Re: svn commit: r1897460 - in /httpd/httpd/trunk: include/ modules/core/ modules/http2/ modules/ssl/ server/ server/mpm/event/ server/mpm/prefork/ server/mpm/winnt/ server/mpm/worker/

2022-01-27 Thread Ruediger Pluem



On 1/25/22 6:34 PM, yla...@apache.org wrote:
> Author: ylavic
> Date: Tue Jan 25 17:34:57 2022
> New Revision: 1897460
> 
> URL: http://svn.apache.org/viewvc?rev=1897460=rev
> Log:
> core: Efficient ap_thread_current() when apr_thread_local() is missing.
> 
> #define ap_thread_create, ap_thread_current_create and ap_thread_current to
> their apr-1.8+ equivalent if available, or implement them using the compiler's
> thread_local mechanism if available, or finally provide stubs otherwise.
> 
> #define AP_HAS_THREAD_LOCAL to 1 in the two former case or 0 otherwise, while
> AP_THREAD_LOCAL is defined to the compiler's keyword iff AP_HAS_THREAD_LOCAL.
> 
> Replace all apr_thread_create() calls with ap_thread_create() so that httpd
> threads can use ap_thread_current()'s pool data as Thread Local Storage.
> 
> Bump MMN minor.
> 
> * include/httpd.h():
>   Define AP_HAS_THREAD_LOCAL, AP_THREAD_LOCAL (eventually), 
> ap_thread_create(),
>   ap_thread_current_create() and ap_thread_current().
>   
> * server/util.c:
>   Implement ap_thread_create(), ap_thread_current_create() and
>   ap_thread_current() when APR < 1.8.
> 
> * modules/core/mod_watchdog.c, modules/http2/h2_workers.c,
> modules/ssl/mod_ssl_ct.c:
>   Use ap_thread_create() instead of apr_thread_create.
> 
> * server/main.c:
>   Use AP_HAS_THREAD_LOCAL and ap_thread_current_create instead of APR's.
> 
> * server/util_pcre.c:
>   Use AP_HAS_THREAD_LOCAL and ap_thread_current instead of APR's.
> 
> * server/mpm/event/event.c, server/mpm/worker/worker.c,
> server/mpm/prefork/prefork.c:
>   Use ap_thread_create() instead of apr_thread_create.
>   Create an apr_thread_t/ap_thread_current() for the main chaild thread usable
>   at child_init().
>   
> * server/mpm/winnt/child.c:
>   Use ap_thread_create() instead of CreateThread().
>   Create an apr_thread_t/ap_thread_current() for the main chaild thread usable
> 
> 
> Modified:
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/httpd.h
> httpd/httpd/trunk/modules/core/mod_watchdog.c
> httpd/httpd/trunk/modules/http2/h2_workers.c
> httpd/httpd/trunk/modules/ssl/mod_ssl_ct.c
> httpd/httpd/trunk/server/main.c
> httpd/httpd/trunk/server/mpm/event/event.c
> httpd/httpd/trunk/server/mpm/prefork/prefork.c
> httpd/httpd/trunk/server/mpm/winnt/child.c
> httpd/httpd/trunk/server/mpm/worker/worker.c
> httpd/httpd/trunk/server/util.c
> httpd/httpd/trunk/server/util_pcre.c
> 

> 
> Modified: httpd/httpd/trunk/server/mpm/event/event.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1897460=1897459=1897460=diff
> ==
> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> +++ httpd/httpd/trunk/server/mpm/event/event.c Tue Jan 25 17:34:57 2022

> @@ -2880,6 +2880,15 @@ static void join_start_thread(apr_thread
>  }
>  }
>  
> +#if AP_HAS_THREAD_LOCAL
> +static apr_status_t main_thread_cleanup(void *arg)
> +{
> +apr_thread_t *thd = arg;
> +apr_pool_destroy(apr_thread_pool_get(thd));
> +return APR_SUCCESS;
> +}
> +#endif
> +
>  static void child_main(int child_num_arg, int child_bucket)
>  {
>  apr_thread_t **threads;
> @@ -2902,6 +2911,28 @@ static void child_main(int child_num_arg
>  apr_pool_create(, pconf);
>  apr_pool_tag(pchild, "pchild");
>  
> +#if AP_HAS_THREAD_LOCAL
> +/* Create an apr_thread_t for the main child thread to set up its
> + * Thread Local Storage. Since it's detached and it won't
> + * apr_thread_exit(), destroy its pool before exiting via
> + * a pchild cleanup
> + */
> +if (!one_process) {
> +apr_thread_t *main_thd = NULL;
> +apr_threadattr_t *main_thd_attr = NULL;
> +if (apr_threadattr_create(_thd_attr, pchild)
> +|| apr_threadattr_detach_set(main_thd_attr, 1)
> +|| ap_thread_current_create(_thd, main_thd_attr,
> +pchild)) {
> +ap_log_error(APLOG_MARK, APLOG_EMERG, 0, ap_server_conf, 
> APLOGNO()
> + "Couldn't initialize child main thread");
> +clean_child_exit(APEXIT_CHILDFATAL);
> +}
> +apr_pool_cleanup_register(pchild, main_thd, main_thread_cleanup,
> +  apr_pool_cleanup_null);
> +}
> +#endif
> +
>  /* close unused listeners and pods */
>  for (i = 0; i < retained->mpm->num_buckets; i++) {
>  if (i != child_bucket) {

Would it make sense to move the above code to mpm_common.c as it looks very 
similar for each MPM?

Regards

RĂ¼diger