You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2005/05/06 18:57:57 UTC

Subversion 1.2.0 RC3 (final candidate) on Monday?

What would we say to rolling rc3 on Monday?  Ben Reser, does that work
for you?  That would give us the weekend to vote on pending STATUS
items (of which there are currently 10, but only 2 or 3 have any
complexity to them).

Assuming no showstoppers, rc3 would be the final candidate,
essentially identical to 1.2.0, which would be released one week
later.  Of course, were we to find a bug in rc3 severe enough to
warrant rc4, that would re-start the 1-week resoak.

Thoughts?
-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Saturday, May 7, 2005 12:10 PM -0500 Ben Collins-Sussman 
<su...@collab.net> wrote:

> I'm little annoyed at the 2-3 day delays we've been having between
> rolling and announcing the tarballs;  maybe that's just the price of  our
> new signature-policy.  But my impression is that we seem to get  three

Yes; but I think that introducing the delay is a good thing...  =)

> signatures for the .gz and .bz2 tarballs within a few hours,  and it's
> the win32 .zip files that are causing the multi-day delays.   Maybe we
> just don't have enough people who can build/test on  Windows?  At the
> least, maybe Branko, Erik and I can prepare our  Windows systems ahead of
> time so that we know we're ready to run  tests on Monday night...?

I'd be fine with announcing once we have 3 +1s overall - regardless of the 
format that has been signed.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Branko Čibej <br...@xbc.nu>.
Ben Collins-Sussman wrote:

>
> On May 7, 2005, at 12:03 PM, Ben Reser wrote:
>
>> On Fri, May 06, 2005 at 01:57:57PM -0500, kfogel@collab.net wrote:
>>
>>> What would we say to rolling rc3 on Monday?  Ben Reser, does that  work
>>> for you?  That would give us the weekend to vote on pending STATUS
>>> items (of which there are currently 10, but only 2 or 3 have any
>>> complexity to them).
>>>
>>> Assuming no showstoppers, rc3 would be the final candidate,
>>> essentially identical to 1.2.0, which would be released one week
>>> later.  Of course, were we to find a bug in rc3 severe enough to
>>> warrant rc4, that would re-start the 1-week resoak.
>>>
>>
>> Monday evening works for me.
>>
>
> Great.  So if breser posts tarballs on Monday evening (U.S.  
> timezones), then hopefully we'll have enough signatures to announce  
> rc3 sometime on Tuesday.
>
> I'm little annoyed at the 2-3 day delays we've been having between  
> rolling and announcing the tarballs;  maybe that's just the price of  
> our new signature-policy.  But my impression is that we seem to get  
> three signatures for the .gz and .bz2 tarballs within a few hours,  
> and it's the win32 .zip files that are causing the multi-day delays.   
> Maybe we just don't have enough people who can build/test on  
> Windows?  At the least, maybe Branko, Erik and I can prepare our  
> Windows systems ahead of time so that we know we're ready to run  
> tests on Monday night...?

I think it's the lack of Windows developers that's delaying us, yes. 
Unfortunately, nobody else seems to be active right now. The TSVN guys 
seem not to be using our tarballs, so we can't get help from there. 
Maybe living with two sigs for the .zip would be enough...

Anyway, aren't Erik and you using the same box for testing? Do you test 
once and sign twice?

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Ben Collins-Sussman <su...@collab.net>.
On May 7, 2005, at 12:03 PM, Ben Reser wrote:

> On Fri, May 06, 2005 at 01:57:57PM -0500, kfogel@collab.net wrote:
>
>> What would we say to rolling rc3 on Monday?  Ben Reser, does that  
>> work
>> for you?  That would give us the weekend to vote on pending STATUS
>> items (of which there are currently 10, but only 2 or 3 have any
>> complexity to them).
>>
>> Assuming no showstoppers, rc3 would be the final candidate,
>> essentially identical to 1.2.0, which would be released one week
>> later.  Of course, were we to find a bug in rc3 severe enough to
>> warrant rc4, that would re-start the 1-week resoak.
>>
>
> Monday evening works for me.
>

Great.  So if breser posts tarballs on Monday evening (U.S.  
timezones), then hopefully we'll have enough signatures to announce  
rc3 sometime on Tuesday.

I'm little annoyed at the 2-3 day delays we've been having between  
rolling and announcing the tarballs;  maybe that's just the price of  
our new signature-policy.  But my impression is that we seem to get  
three signatures for the .gz and .bz2 tarballs within a few hours,  
and it's the win32 .zip files that are causing the multi-day delays.   
Maybe we just don't have enough people who can build/test on  
Windows?  At the least, maybe Branko, Erik and I can prepare our  
Windows systems ahead of time so that we know we're ready to run  
tests on Monday night...?


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Ben Reser <be...@reser.org>.
On Fri, May 06, 2005 at 01:57:57PM -0500, kfogel@collab.net wrote:
> What would we say to rolling rc3 on Monday?  Ben Reser, does that work
> for you?  That would give us the weekend to vote on pending STATUS
> items (of which there are currently 10, but only 2 or 3 have any
> complexity to them).
> 
> Assuming no showstoppers, rc3 would be the final candidate,
> essentially identical to 1.2.0, which would be released one week
> later.  Of course, were we to find a bug in rc3 severe enough to
> warrant rc4, that would re-start the 1-week resoak.

Monday evening works for me.

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Ben Collins-Sussman <su...@collab.net>.
On May 6, 2005, at 5:14 PM, Branko Čibej wrote:

> kfogel@collab.net wrote:
>
>
>> What would we say to rolling rc3 on Monday?  Ben Reser, does that  
>> work
>> for you?  That would give us the weekend to vote on pending STATUS
>> items (of which there are currently 10, but only 2 or 3 have any
>> complexity to them).
>>
>> Assuming no showstoppers, rc3 would be the final candidate,
>> essentially identical to 1.2.0, which would be released one week
>> later.  Of course, were we to find a bug in rc3 severe enough to
>> warrant rc4, that would re-start the 1-week resoak.
>>
>> Thoughts?
>>
>>
> I think we have to fix the propchange-commit bug that Stefan found  
> before rolling RC3. Otherwise, +1.
>

Fear not, I'm hot on the trail.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Branko Čibej <br...@xbc.nu>.
Branko Čibej wrote:

> kfogel@collab.net wrote:
>
>> What would we say to rolling rc3 on Monday?  Ben Reser, does that work
>> for you?  That would give us the weekend to vote on pending STATUS
>> items (of which there are currently 10, but only 2 or 3 have any
>> complexity to them).
>>
>> Assuming no showstoppers, rc3 would be the final candidate,
>> essentially identical to 1.2.0, which would be released one week
>> later.  Of course, were we to find a bug in rc3 severe enough to
>> warrant rc4, that would re-start the 1-week resoak.
>>
>> Thoughts?
>>  
>>
> I think we have to fix the propchange-commit bug that Stefan found 
> before rolling RC3. Otherwise, +1.

Oh, and what about the hotcopy problems D.J. Heap describes? I'm 
positive we want those fixed in 1.2.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:

>What would we say to rolling rc3 on Monday?  Ben Reser, does that work
>for you?  That would give us the weekend to vote on pending STATUS
>items (of which there are currently 10, but only 2 or 3 have any
>complexity to them).
>
>Assuming no showstoppers, rc3 would be the final candidate,
>essentially identical to 1.2.0, which would be released one week
>later.  Of course, were we to find a bug in rc3 severe enough to
>warrant rc4, that would re-start the 1-week resoak.
>
>Thoughts?
>  
>
I think we have to fix the propchange-commit bug that Stefan found 
before rolling RC3. Otherwise, +1.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Michael Sinz <mi...@gmail.com>.
On 5/8/05, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> On Sun, 8 May 2005, Patrick Mayweg wrote:
> 
> > I have researched the problem in detail. The java executable on windows
> > uses just 256KB of stack size instead of the default 1MB stack size. In
> > this situation putting buffers of 100KB (SVN_STREAM_CHUNK_SIZE) becomes
> > a problem. There are some functions putting 1 or 2 buffers on the stack.
> > Other functions getting this kind of buffer space from an apr pool. To
> > solve Marks problem, we could either reduce the buffer size to 90KB or
> > take the space from  the pool. Since both ways are out of my commit
> > area, I would like to have some input, which way to go for working out a
> > patch.
> 
> Ouch! Putting vars of those sizes on the stack isn't very friendly. Just
> allocate it out of a pool instead.

I agree - very bad stack manners.

-- 
Michael Sinz               Technology and Engineering Director/Consultant
"Starting Startups"                          mailto:Michael.Sinz@sinz.org
My place on the web                      http://www.sinz.org/Michael.Sinz

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Erik Huelsmann <eh...@gmail.com>.
On 5/8/05, Patrick Mayweg <ma...@qint.de> wrote:
> Hi Philip,
> 
> Philip Martin wrote:
> 
> >"Peter N. Lundblad" <pe...@famlundblad.se> writes:
> >
> >
> >
> >>Ouch! Putting vars of those sizes on the stack isn't very friendly. Just
> >>allocate it out of a pool instead.
> >>
> >>(Saying this without having investigated in detail, but I can't imagine it
> >>should be a problem.)
> >>
> >>
> >
> >Look at this:
> >
> >
> This is the only case, where a pool is not available.
> 
> >svn_error_t *
> >svn_subst_translate_stream (svn_stream_t *s, /* src stream */
> >                            svn_stream_t *d, /* dst stream */
> >                            const char *eol_str,
> >                            svn_boolean_t repair,
> >                            const svn_subst_keywords_t *keywords,
> >                            svn_boolean_t expand)
> >{
> >  char buf[SVN_STREAM_CHUNK_SIZE + 1];
> >  const char *p, *interesting;
> >
> >There is no readily available pool, and neither svn_stream_t nor
> >svn_subst_keywords_t provides one.  There is a void* baton in
> >svn_stream_t but svn_subst_translate_stream can't really make any
> >assumptions about it.
> >
> >
> All callers of this function have a pool available. If  I could extend
> the interface by a pool parameter, I could change that case too.
> Patrick

Looking at the name of the function, that will need to happen before
1.2 final, since it seems like a public function. If we need a revved
API, it has to be done in a minor release.

We could ofcourse update all internal callers, but it won't fix the
problem for external callers.

bye,


Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: [PATCH 2] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Patrick Mayweg <ma...@qint.de>.
Hi  Ben,

Ben Collins-Sussman wrote:

>
> On May 8, 2005, at 9:04 AM, Patrick Mayweg wrote:
>
>>
>> * subversion/include/svn_subst.h
>>  (svn_subst_translate_stream) : add new apr_pool parameter.
>>
>
> The spirit of this patch is good, but it's illegal (by our  
> compatibility rules) to change the interface of an already-released  
> public API.
>
> What you need to do is create a new svn_subst_translate_stream2(),  
> and make the old API just a dumb-wrapper around the new API.
>
I would have liked to do that, but where to get the pool, if you do not 
have one?
Patrick

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH 2] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Ben Collins-Sussman <su...@collab.net>.
On May 8, 2005, at 9:04 AM, Patrick Mayweg wrote:
>
> * subversion/include/svn_subst.h
>  (svn_subst_translate_stream) : add new apr_pool parameter.
>

The spirit of this patch is good, but it's illegal (by our  
compatibility rules) to change the interface of an already-released  
public API.

What you need to do is create a new svn_subst_translate_stream2(),  
and make the old API just a dumb-wrapper around the new API.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH 2] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Patrick Mayweg <ma...@qint.de>.
Hi Erik,

Erik Huelsmann wrote:

>>Patrick Mayweg wrote:
>>    
>>
>>>>Hi,
>>>>this patch removes the last of the SVN_STREAM_CHUNK_SIZE buffers from
>>>>the stack and use the pool instead. This fixes the stack overflow for
>>>>the javahl binding on windows. I am not sure if this change is
>>>>permissible, because of the interface change.
>>>>        
>>>>
>>No, it is not, for exactly this reason.
>>    
>>
>
>But the API *can* be revved...
>
>Which seems to be the right thing to do if it actually is a showstopper to
>use bindings on an otherwise perfectly supported platform.
>  
>
If my first patch would be committed, that would get the javahl binding 
on windows running again. That decreases the stack needed enough for the 
time beeing.

>  
>
>>Max.
>>    
>>
>
>bye,
>
>Erik.
>
>  
>
Regards,
Patrick

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH 2] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Erik Huelsmann <e....@gmx.net>.
> Patrick Mayweg wrote:
> >> Hi,
> >> this patch removes the last of the SVN_STREAM_CHUNK_SIZE buffers from
> >> the stack and use the pool instead. This fixes the stack overflow for
> >> the javahl binding on windows. I am not sure if this change is
> >> permissible, because of the interface change.
> 
> No, it is not, for exactly this reason.

But the API *can* be revved...

Which seems to be the right thing to do if it actually is a showstopper to
use bindings on an otherwise perfectly supported platform.

> Max.

bye,

Erik.

-- 
+++ Neu: Echte DSL-Flatrates von GMX - Surfen ohne Limits +++
Always online ab 4,99 Euro/Monat: http://www.gmx.net/de/go/dsl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH 2] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Max Bowsher <ma...@ukf.net>.
Patrick Mayweg wrote:
>> Hi,
>> this patch removes the last of the SVN_STREAM_CHUNK_SIZE buffers from
>> the stack and use the pool instead. This fixes the stack overflow for
>> the javahl binding on windows. I am not sure if this change is
>> permissible, because of the interface change.

No, it is not, for exactly this reason.

Max.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH 2] V2 Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Patrick Mayweg <ma...@qint.de>.
Hi Jani,
thanks again for pointing that out. I have made a new patch.
Regards,
Patrick

Log message:
[[[
Use the pool memory instead of stack memory for big buffers in
svn_subst_translate_stream, which needs a new parameter.

* subversion/include/svn_subst.h
 (svn_subst_translate_stream) : add new apr_pool parameter.

* subversion/libsvn_subr/subst.c
 (svn_subst_translate_stream) : add new apr_pool parameter and use it 
allocate
  a big buffer from that pool.
 (svn_subst_copy_and_translate2, svn_subst_copy_and_translate) : add pool
  parameter to svn_subst_translate_stream call.

* subversion/libsvn_wc/props.c
 (validate_eol_prop_against_file) : add pool parameter to
  svn_subst_translate_stream call.

* subversion/libsvn_client/cat.c
 (cat_local_file,svn_client_cat2) : add pool parameter to
  svn_subst_translate_stream call.
]]]


Jani Averbach wrote:

>On 2005-05-08 16:04+0200, Patrick Mayweg wrote:
>
>  
>
>>Index: subversion/libsvn_subr/subst.c
>>===================================================================
>>--- subversion/libsvn_subr/subst.c	(revision 14544)
>>+++ subversion/libsvn_subr/subst.c	(working copy)
>>@@ -591,9 +591,10 @@
>>                             const char *eol_str,
>>                             svn_boolean_t repair,
>>                             const svn_subst_keywords_t *keywords,
>>-                            svn_boolean_t expand)
>>+                            svn_boolean_t expand,
>>+                            apr_pool_t *pool)
>> {
>>-  char buf[SVN_STREAM_CHUNK_SIZE + 1];
>>+  char *buf = apr_palloc(pool, SVN_STREAM_CHUNK_SIZE + 1);
>>   const char *p, *interesting;
>>   apr_size_t len, readlen;
>>   apr_size_t eol_str_len = eol_str ? strlen (eol_str) : 0;
>>    
>>
>
>Same thing here, sizeof used with dynamically allocated buffer.
>  ...
>  readlen = sizeof (buf) - 1;
>  while (readlen == sizeof (buf) - 1)
>   ...
>
>BR, Jani
>
>  
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH 2] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Jani Averbach <ja...@jaa.iki.fi>.
On 2005-05-08 16:04+0200, Patrick Mayweg wrote:

> Index: subversion/libsvn_subr/subst.c
> ===================================================================
> --- subversion/libsvn_subr/subst.c	(revision 14544)
> +++ subversion/libsvn_subr/subst.c	(working copy)
> @@ -591,9 +591,10 @@
>                              const char *eol_str,
>                              svn_boolean_t repair,
>                              const svn_subst_keywords_t *keywords,
> -                            svn_boolean_t expand)
> +                            svn_boolean_t expand,
> +                            apr_pool_t *pool)
>  {
> -  char buf[SVN_STREAM_CHUNK_SIZE + 1];
> +  char *buf = apr_palloc(pool, SVN_STREAM_CHUNK_SIZE + 1);
>    const char *p, *interesting;
>    apr_size_t len, readlen;
>    apr_size_t eol_str_len = eol_str ? strlen (eol_str) : 0;

Same thing here, sizeof used with dynamically allocated buffer.
  ...
  readlen = sizeof (buf) - 1;
  while (readlen == sizeof (buf) - 1)
   ...

BR, Jani

-- 
Jani Averbach


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

[PATCH 2] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Patrick Mayweg <ma...@qint.de>.
Hi,
this patch removes the last of the SVN_STREAM_CHUNK_SIZE buffers from 
the stack and use the pool instead. This fixes the stack overflow for 
the javahl binding on windows. I am not sure if this change is 
permissible, because of the interface change.
Regards,
Patrick Mayweg

Log message:
12345678901234567890123456789012345678901234567890123456789012345678901234567890 

[[[
Use the pool memory instead of stack memory for big buffers in
svn_subst_translate_stream, which needs a new parameter.

* subversion/include/svn_subst.h
  (svn_subst_translate_stream) : add new apr_pool parameter.

* subversion/libsvn_subr/subst.c
  (svn_subst_translate_stream) : add new apr_pool parameter and use it 
allocate
   a big buffer from that pool.
  (svn_subst_copy_and_translate2, svn_subst_copy_and_translate) : add pool
   parameter to svn_subst_translate_stream call.

* subversion/libsvn_wc/props.c
  (validate_eol_prop_against_file) : add pool parameter to
   svn_subst_translate_stream call.

* subversion/libsvn_client/cat.c
  (cat_local_file,svn_client_cat2) : add pool parameter to
   svn_subst_translate_stream call.
]]]




Patrick Mayweg wrote:

> Hi Philip,
>
> Philip Martin wrote:
>
>> "Peter N. Lundblad" <pe...@famlundblad.se> writes:
>>
>>  
>>
>>> Ouch! Putting vars of those sizes on the stack isn't very friendly. 
>>> Just
>>> allocate it out of a pool instead.
>>>
>>> (Saying this without having investigated in detail, but I can't 
>>> imagine it
>>> should be a problem.)
>>>   
>>
>>
>> Look at this:
>>  
>>
> This is the only case, where a pool is not available.
>
>> svn_error_t *
>> svn_subst_translate_stream (svn_stream_t *s, /* src stream */
>>                            svn_stream_t *d, /* dst stream */
>>                            const char *eol_str,
>>                            svn_boolean_t repair,
>>                            const svn_subst_keywords_t *keywords,
>>                            svn_boolean_t expand)
>> {
>>  char buf[SVN_STREAM_CHUNK_SIZE + 1];
>>  const char *p, *interesting;
>>
>> There is no readily available pool, and neither svn_stream_t nor
>> svn_subst_keywords_t provides one.  There is a void* baton in
>> svn_stream_t but svn_subst_translate_stream can't really make any
>> assumptions about it.
>>  
>>
> All callers of this function have a pool available. If  I could extend 
> the interface by a pool parameter, I could change that case too.
> Patrick
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>

Re: [PATCH 1] V2 Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Patrick Mayweg <ma...@qint.de>.
Hi Jani,
thanks for pointing that out. I have made a new patch.
Regards,
Patrick

Log message:
[[[
Use the pool memory instead of stack memory for big buffers in all 
cases, which
do not need an interface change.

* subversion/libsvn_ra_dav/util.c
 (parse_spool_file) : use pool memory for big buffer.

* subversion/libsvn_repos/reporter.c
 (compare_files) : use pool memory for big buffers.

* subversion/libsvn_repos/delta.c
 (compare_files) : use pool memory for big buffers.

* subversion/libsvn_subr/stream.c
 (svn_stream_copy) : use pool memory for big buffer.
]]]

Jani Averbach wrote:

>On 2005-05-08 16:03+0200, Patrick Mayweg wrote:
>  
>
>>Hi,
>>this patch removes all but one of the SVN_STREAM_CHUNK_SIZE buffers from 
>>the stack and use the pool instead. This fixes the stack overflow for 
>>the javahl binding on windows.
>>    
>>
>
>I have two comments:
>
>  
>
>>Index: subversion/libsvn_ra_dav/util.c
>>===================================================================
>>--- subversion/libsvn_ra_dav/util.c	(revision 14544)
>>+++ subversion/libsvn_ra_dav/util.c	(working copy)
>>@@ -502,7 +502,7 @@
>> {
>>   apr_file_t *spool_file;
>>   svn_stream_t *spool_stream;
>>-  char buf[SVN_STREAM_CHUNK_SIZE];
>>+  char *buf = apr_palloc(pool, SVN_STREAM_CHUNK_SIZE);
>>   apr_size_t len;
>>   
>>   SVN_ERR( svn_io_file_open(&spool_file, spool_file_name,
>>    
>>
>
>Later in the same function there is code like this:
>
>      len = sizeof (buf);
>      ...
>      if (len != sizeof (buf))
>
>This won't work with dynamically allocated buf.
>
>
>  
>
>>Index: subversion/libsvn_subr/stream.c
>>===================================================================
>>--- subversion/libsvn_subr/stream.c	(revision 14544)
>>+++ subversion/libsvn_subr/stream.c	(working copy)
>>@@ -183,7 +183,7 @@
>> svn_error_t *svn_stream_copy (svn_stream_t *from, svn_stream_t *to,
>>                               apr_pool_t *pool)
>> {
>>-  char buf[SVN_STREAM_CHUNK_SIZE];
>>+  char *buf = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE);
>>   apr_size_t len;
>> 
>>    
>>
>
>And same goes here:
>
>  while (1)
>    {
>      len = sizeof (buf);
>      ...
>
>BR, Jani
>
>  
>

Re: [PATCH 1] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Jani Averbach <ja...@jaa.iki.fi>.
On 2005-05-08 16:03+0200, Patrick Mayweg wrote:
> Hi,
> this patch removes all but one of the SVN_STREAM_CHUNK_SIZE buffers from 
> the stack and use the pool instead. This fixes the stack overflow for 
> the javahl binding on windows.

I have two comments:

> Index: subversion/libsvn_ra_dav/util.c
> ===================================================================
> --- subversion/libsvn_ra_dav/util.c	(revision 14544)
> +++ subversion/libsvn_ra_dav/util.c	(working copy)
> @@ -502,7 +502,7 @@
>  {
>    apr_file_t *spool_file;
>    svn_stream_t *spool_stream;
> -  char buf[SVN_STREAM_CHUNK_SIZE];
> +  char *buf = apr_palloc(pool, SVN_STREAM_CHUNK_SIZE);
>    apr_size_t len;
>    
>    SVN_ERR( svn_io_file_open(&spool_file, spool_file_name,

Later in the same function there is code like this:

      len = sizeof (buf);
      ...
      if (len != sizeof (buf))

This won't work with dynamically allocated buf.


> Index: subversion/libsvn_subr/stream.c
> ===================================================================
> --- subversion/libsvn_subr/stream.c	(revision 14544)
> +++ subversion/libsvn_subr/stream.c	(working copy)
> @@ -183,7 +183,7 @@
>  svn_error_t *svn_stream_copy (svn_stream_t *from, svn_stream_t *to,
>                                apr_pool_t *pool)
>  {
> -  char buf[SVN_STREAM_CHUNK_SIZE];
> +  char *buf = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE);
>    apr_size_t len;
>  

And same goes here:

  while (1)
    {
      len = sizeof (buf);
      ...

BR, Jani

-- 
Jani Averbach

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

[PATCH 1] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Patrick Mayweg <ma...@qint.de>.
Hi,
this patch removes all but one of the SVN_STREAM_CHUNK_SIZE buffers from 
the stack and use the pool instead. This fixes the stack overflow for 
the javahl binding on windows.
Regards,
Patrick Mayweg

Log message:
[[[
Use the pool memory instead of stack memory for big buffers in all 
cases, which
do not need an interface change.

* subversion/libsvn_ra_dav/util.c
  (parse_spool_file) : use pool memory for big buffer.

* subversion/libsvn_repos/reporter.c
  (compare_files) : use pool memory for big buffers.

* subversion/libsvn_repos/delta.c
  (compare_files) : use pool memory for big buffers.

* subversion/libsvn_subr/stream.c
  (svn_stream_copy) : use pool memory for big buffer.
]]]

Patrick Mayweg wrote:

> Hi Philip,
>
> Philip Martin wrote:
>
>> "Peter N. Lundblad" <pe...@famlundblad.se> writes:
>>
>>  
>>
>>> Ouch! Putting vars of those sizes on the stack isn't very friendly. 
>>> Just
>>> allocate it out of a pool instead.
>>>
>>> (Saying this without having investigated in detail, but I can't 
>>> imagine it
>>> should be a problem.)
>>>   
>>
>>
>>
>> Look at this:
>>  
>>
> This is the only case, where a pool is not available.
>
>> svn_error_t *
>> svn_subst_translate_stream (svn_stream_t *s, /* src stream */
>>                            svn_stream_t *d, /* dst stream */
>>                            const char *eol_str,
>>                            svn_boolean_t repair,
>>                            const svn_subst_keywords_t *keywords,
>>                            svn_boolean_t expand)
>> {
>>  char buf[SVN_STREAM_CHUNK_SIZE + 1];
>>  const char *p, *interesting;
>>
>> There is no readily available pool, and neither svn_stream_t nor
>> svn_subst_keywords_t provides one.  There is a void* baton in
>> svn_stream_t but svn_subst_translate_stream can't really make any
>> assumptions about it.
>>  
>>
> All callers of this function have a pool available. If  I could extend 
> the interface by a pool parameter, I could change that case too.
> Patrick
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Patrick Mayweg <ma...@qint.de>.
Hi Philip,

Philip Martin wrote:

>"Peter N. Lundblad" <pe...@famlundblad.se> writes:
>
>  
>
>>Ouch! Putting vars of those sizes on the stack isn't very friendly. Just
>>allocate it out of a pool instead.
>>
>>(Saying this without having investigated in detail, but I can't imagine it
>>should be a problem.)
>>    
>>
>
>Look at this:
>  
>
This is the only case, where a pool is not available.

>svn_error_t *
>svn_subst_translate_stream (svn_stream_t *s, /* src stream */
>                            svn_stream_t *d, /* dst stream */
>                            const char *eol_str,
>                            svn_boolean_t repair,
>                            const svn_subst_keywords_t *keywords,
>                            svn_boolean_t expand)
>{
>  char buf[SVN_STREAM_CHUNK_SIZE + 1];
>  const char *p, *interesting;
>
>There is no readily available pool, and neither svn_stream_t nor
>svn_subst_keywords_t provides one.  There is a void* baton in
>svn_stream_t but svn_subst_translate_stream can't really make any
>assumptions about it.
>  
>
All callers of this function have a pool available. If  I could extend 
the interface by a pool parameter, I could change that case too.
Patrick

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Philip Martin <ph...@codematters.co.uk>.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:

> Ouch! Putting vars of those sizes on the stack isn't very friendly. Just
> allocate it out of a pool instead.
>
> (Saying this without having investigated in detail, but I can't imagine it
> should be a problem.)

Look at this:

svn_error_t *
svn_subst_translate_stream (svn_stream_t *s, /* src stream */
                            svn_stream_t *d, /* dst stream */
                            const char *eol_str,
                            svn_boolean_t repair,
                            const svn_subst_keywords_t *keywords,
                            svn_boolean_t expand)
{
  char buf[SVN_STREAM_CHUNK_SIZE + 1];
  const char *p, *interesting;

There is no readily available pool, and neither svn_stream_t nor
svn_subst_keywords_t provides one.  There is a void* baton in
svn_stream_t but svn_subst_translate_stream can't really make any
assumptions about it.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 8 May 2005, Patrick Mayweg wrote:

> I have researched the problem in detail. The java executable on windows
> uses just 256KB of stack size instead of the default 1MB stack size. In
> this situation putting buffers of 100KB (SVN_STREAM_CHUNK_SIZE) becomes
> a problem. There are some functions putting 1 or 2 buffers on the stack.
> Other functions getting this kind of buffer space from an apr pool. To
> solve Marks problem, we could either reduce the buffer size to 90KB or
> take the space from  the pool. Since both ways are out of my commit
> area, I would like to have some input, which way to go for working out a
> patch.

Ouch! Putting vars of those sizes on the stack isn't very friendly. Just
allocate it out of a pool instead.

(Saying this without having investigated in detail, but I can't imagine it
should be a problem.)

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Patrick Mayweg <ma...@qint.de>.
Hi Mark,
I have researched the problem in detail. The java executable on windows 
uses just 256KB of stack size instead of the default 1MB stack size. In 
this situation putting buffers of 100KB (SVN_STREAM_CHUNK_SIZE) becomes 
a problem. There are some functions putting 1 or 2 buffers on the stack. 
Other functions getting this kind of buffer space from an apr pool. To 
solve Marks problem, we could either reduce the buffer size to 90KB or 
take the space from  the pool. Since both ways are out of my commit 
area, I would like to have some input, which way to go for working out a 
patch.
Regards,
Patrick


Mark Phippard wrote:

>kfogel@collab.net wrote on 05/06/2005 02:57:57 PM:
>
>  
>
>>What would we say to rolling rc3 on Monday?  Ben Reser, does that work
>>for you?  That would give us the weekend to vote on pending STATUS
>>items (of which there are currently 10, but only 2 or 3 have any
>>complexity to them).
>>
>>Assuming no showstoppers, rc3 would be the final candidate,
>>essentially identical to 1.2.0, which would be released one week
>>later.  Of course, were we to find a bug in rc3 severe enough to
>>warrant rc4, that would re-start the 1-week resoak.
>>    
>>
>
>I guess it is not a show stopper, but recall that I have a pending issue 
>that merge over http:// from the JavaHL binding crashes the JVM on 
>Windows.  Patrick Mayweg is planning to look at it when he gets home from 
>vacation this weekend.  He did some preliminary checking and this looks 
>like it might be a tough problem to solve.  The issue is that Java uses a 
>really small stack size on Windows and there is no reasonable way to alter 
>it or control it.  Some changes in the merge API have apparently caused 
>the stack usage to exceed this limit.  Unless there is some way to compile 
>the JavaHL DLL so that it allocates all memory from the heap, I do not 
>know how this could be fixed.
>
>Hopefully Patrick will find some clever solution. 
>
>Just wanted you to be aware of it.  I am really not sure what we are doing 
>to do about this in Subclipse.  It is pretty nasty to just crash the 
>user's IDE.
>
>Mark
>
>
>_____________________________________________________________________________
>Scanned for SoftLanding Systems, Inc. by IBM Email Security Management Services powered by MessageLabs. 
>_____________________________________________________________________________
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>For additional commands, e-mail: dev-help@subversion.tigris.org
>
>  
>

Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

Posted by Mark Phippard <Ma...@softlanding.com>.
kfogel@collab.net wrote on 05/06/2005 02:57:57 PM:

> What would we say to rolling rc3 on Monday?  Ben Reser, does that work
> for you?  That would give us the weekend to vote on pending STATUS
> items (of which there are currently 10, but only 2 or 3 have any
> complexity to them).
> 
> Assuming no showstoppers, rc3 would be the final candidate,
> essentially identical to 1.2.0, which would be released one week
> later.  Of course, were we to find a bug in rc3 severe enough to
> warrant rc4, that would re-start the 1-week resoak.

I guess it is not a show stopper, but recall that I have a pending issue 
that merge over http:// from the JavaHL binding crashes the JVM on 
Windows.  Patrick Mayweg is planning to look at it when he gets home from 
vacation this weekend.  He did some preliminary checking and this looks 
like it might be a tough problem to solve.  The issue is that Java uses a 
really small stack size on Windows and there is no reasonable way to alter 
it or control it.  Some changes in the merge API have apparently caused 
the stack usage to exceed this limit.  Unless there is some way to compile 
the JavaHL DLL so that it allocates all memory from the heap, I do not 
know how this could be fixed.

Hopefully Patrick will find some clever solution. 

Just wanted you to be aware of it.  I am really not sure what we are doing 
to do about this in Subclipse.  It is pretty nasty to just crash the 
user's IDE.

Mark


_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. by IBM Email Security Management Services powered by MessageLabs. 
_____________________________________________________________________________

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org