Hi Chris,

The code includes Connection: Keep-Alive header to support servers that 
implement HTTP/1.0 only.

If you mean that these changes are unnecessary, because HTTP/1.1 already 
defaults to persistent connections, then no, they are not. With the 
current implementation, the client will close the socket once the request 
has completed, and it will open a new connection for each subsequent 
request.

If you think the speedup is negligible, then I suggest you backport it to 
a 5.1 (or older) image (really easy, just file out the changes between 
Monticello-cmm.677 and Monticello-ul.678 and file them into the 5.1 image) 
and measure the difference in updating the image to a certain point (to 
avoid broken updates and merges).

Levente

On Tue, 20 Mar 2018, Chris Muller wrote:

> Hey Levente,
>
> Persistent TCP connections by default was changed from HTTP 1.0 to
> 1.1.  I know this is just for talking to our own server, but do you
> suppose we are following W3C recommendations for proper HTTP
> negotiation in each scenario, or should we need to declare any
> explicit protocol versions or header options for that?
>
>    https://www.w3.org/Protocols/rfc2616/rfc2616-sec8.html#sec8.1.2.1
>
>
> On Sun, Mar 18, 2018 at 7:13 PM,  <commits at source.squeak.org> wrote:
>> Levente Uzonyi uploaded a new version of Monticello to project The Inbox:
>> http://source.squeak.org/inbox/Monticello-ul.678.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Monticello-ul.678
>> Author: ul
>> Time: 19 March 2018, 1:03:18.137853 am
>> UUID: 4d47635e-aa48-43cc-99bb-b8d2f1a5a51f
>> Ancestors: Monticello-cmm.677
>>
>> Store and reuse a shared WebClient instance for each MCHttpRepository. This makes it possible to speed up downloads by reusing TCP connections.
>> Disabled by default. Enable it by evaluating [MCHttpRepository useSharedWebClientInstance: true]
>>
>> =============== Diff against Monticello-cmm.677 ===============
>>
>> Item was changed:
>>   MCFileBasedRepository subclass: #MCHttpRepository
>> +       instanceVariableNames: 'location user password readerCache indexed webClient'
>> +       classVariableNames: 'UseSharedWebClientInstance'
>> -       instanceVariableNames: 'location user password readerCache indexed'
>> -       classVariableNames: ''
>>         poolDictionaries: ''
>>         category: 'Monticello-Repositories'!
>>
>> Item was added:
>> + ----- Method: MCHttpRepository class>>useSharedWebClientInstance (in category 'preferences') -----
>> + useSharedWebClientInstance
>> +
>> +       <preference: 'Use shared WebClient instance'
>> +               category: 'Monticello'
>> +               description: 'When true, use a shared WebClient instance to speed up downloads from MCHttpRepositories. Requires WebClient to be present.'
>> +               type: #Boolean>
>> +       ^UseSharedWebClientInstance ifNil: [ false ]!
>>
>> Item was added:
>> + ----- Method: MCHttpRepository class>>useSharedWebClientInstance: (in category 'preferences') -----
>> + useSharedWebClientInstance: aBoolean
>> +
>> +       UseSharedWebClientInstance := aBoolean!
>>
>> Item was changed:
>>   ----- Method: MCHttpRepository>>allFileNames (in category 'overriding') -----
>>   allFileNames
>> +
>>         | index |
>> +       index := self displayProgress: 'Updating ', self description during: [
>> +               self httpGet: self locationWithTrailingSlash, '?C=M;O=D' arguments: nil ].
>> +       ^index ifNotNil: [ self parseFileNamesFromStream: index ]!
>> -       self displayProgress: 'Updating ', self description during:[
>> -               index := HTTPSocket httpGet: self locationWithTrailingSlash, '?C=M;O=D' args: nil user: self user passwd: self password.
>> -       ].
>> -       index isString ifTrue: [NetworkError signal: 'Could not access ', location].
>> -       ^ self parseFileNamesFromStream: index  !
>>
>> Item was added:
>> + ----- Method: MCHttpRepository>>httpGet:arguments: (in category 'private') -----
>> + httpGet: url arguments: arguments
>> +
>> +       | progress urlString client  response result |
>> +       self class useSharedWebClientInstance ifFalse: [
>> +               ^HTTPSocket httpGet: url args: arguments user: self user passwd: self password ].
>> +       progress := [ :total :amount |
>> +               HTTPProgress new
>> +                       total: total;
>> +                       amount: amount;
>> +                       signal: 'Downloading...' ].
>> +       urlString := arguments
>> +               ifNil: [ url ]
>> +               ifNotNil: [
>> +                       | queryString |
>> +                       queryString := (Smalltalk classNamed: #WebUtils) encodeUrlEncodedForm: arguments.
>> +                       (url includes: $?)
>> +                               ifTrue: [ url, '&', queryString ]
>> +                               ifFalse: [ url, '?', queryString ] ].
>> +       "Acquire webClient by atomically storing it in the client variable and setting its value to nil."
>> +       client := webClient.
>> +       webClient := nil.
>> +       client ifNil: [ client := (Smalltalk classNamed: #WebClient) new ].
>> +       response := client
>> +               username: self user;
>> +               password: self password;
>> +               httpGet: urlString do: [ :request |
>> +                       request
>> +                               headerAt: 'Authorization' put: 'Basic ', (self user, ':', self password) base64Encoded;
>> +                               headerAt: 'Connection' put: 'Keep-Alive';
>> +                               headerAt: 'Accept' put: '*/*' ].
>> +       result := (response code between: 200 and: 299)
>> +               ifFalse: [
>> +                       response content. "Make sure content is read."
>> +                       nil ]
>> +               ifTrue: [ (RWBinaryOrTextStream with: (response contentWithProgress: progress)) reset ].
>> +       "Save the WebClient instance for reuse, but only if there is no client cached."
>> +       webClient
>> +               ifNil: [ webClient := client ]
>> +               ifNotNil: [ client close ].
>> +       ^result ifNil: [
>> +               NetworkError signal: 'Could not access ', location.
>> +               nil ]!
>>
>> Item was changed:
>>   ----- Method: MCHttpRepository>>readStreamForFileNamed:do: (in category 'private') -----
>>   readStreamForFileNamed: aString do: aBlock
>> +
>>         | contents |
>> +       contents := self displayProgress: 'Downloading ', aString during: [
>> +               self httpGet: (self urlForFileNamed: aString) arguments: nil ].
>> +       ^contents ifNotNil: [ aBlock value: contents ]!
>> -       self displayProgress: 'Downloading ', aString during:[
>> -               contents := HTTPSocket httpGet: (self urlForFileNamed: aString) args: nil user: self user passwd: self password.
>> -       ].
>> -       ^ contents isString ifFalse: [aBlock value: contents]!
>>
>>