The Inbox: Monticello-ul.726.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

The Inbox: Monticello-ul.726.mcz

commits-2
Levente Uzonyi uploaded a new version of Monticello to project The Inbox:
http://source.squeak.org/inbox/Monticello-ul.726.mcz

==================== Summary ====================

Name: Monticello-ul.726
Author: ul
Time: 29 June 2020, 1:39:36.986367 am
UUID: b64f6b16-96de-40ed-aa9a-2c0f625bfcb1
Ancestors: Monticello-mt.725

- potentially speed up MCDirectoryRepository >> #includesVersionNamed: when the repository doesn't have version names cached
- cache self size in MCVersionName >> #versionName

=============== Diff against Monticello-mt.725 ===============

Item was added:
+ ----- Method: MCDirectoryRepository>>includesVersionNamed: (in category 'versions') -----
+ includesVersionNamed: aString
+
+ | comparable |
+ comparable := ((aString endsWith: '.mcz') and: [ aString size > 4 ])
+ ifTrue: [ aString allButLast: 4 ]
+ ifFalse: [ aString ].
+ allVersionNamesCache ifNil: [
+ "Instead of reading the contents of the entire directory in #allVersionNames, look up a single .mcz file.
+ This is just an optimization. If the file does not exist, the version may still be there as an mcd."
+ (directory fileExists: comparable, '.mcz') ifTrue: [ ^true ] ].
+ ^ self allVersionNames includes: comparable!

Item was changed:
  ----- Method: MCVersionName>>versionName (in category 'accessing') -----
  versionName
  "Answer my version name as a ByteString, without the file suffix or any ancestor-attributes."
  | end |
  self isEmpty ifTrue: [^ String empty].
  end := self indexOf: $( ifAbsent: [
+ | size |
+ size := self size.
+ (size > 4
+ and: [ (self at: size - 3) == $.
+ and: [ (self at: size - 2) == $m
+ and: [ (self at: size - 1) == $c ] ] ])
+ ifTrue: [size - 3]
+ ifFalse: [size + 1]].
- (self size > 4
- and: [ (self at: self size - 3) == $.
- and: [ (self at: self size - 2) == $m
- and: [ (self at: self size - 1) == $c ] ] ])
- ifTrue: [self size - 3]
- ifFalse: [self size + 1]].
  ^self first: end - 1!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Monticello-ul.726.mcz

Jakob Reschke-2
Am Mo., 29. Juni 2020 um 01:43 Uhr schrieb <[hidden email]>:
>
> + ----- Method: MCDirectoryRepository>>includesVersionNamed: (in category 'versions') -----
> + includesVersionNamed: aString
> +
> +       | comparable |
> +       comparable := ((aString endsWith: '.mcz') and: [ aString size > 4 ])
> +               ifTrue: [ aString allButLast: 4 ]
> +               ifFalse: [ aString ].

Thank you for the suggestion. Why not use aString asMCVersionName
versionName here?

Doesn't the repository include the version even if it is just a diffy
version? You might say no because there is no complete snapshot, but
the diffy version has the same versionInfo after all, doesn't it?

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Monticello-ul.726.mcz

Levente Uzonyi
On Mon, 29 Jun 2020, Jakob Reschke wrote:

> Am Mo., 29. Juni 2020 um 01:43 Uhr schrieb <[hidden email]>:
>>
>> + ----- Method: MCDirectoryRepository>>includesVersionNamed: (in category 'versions') -----
>> + includesVersionNamed: aString
>> +
>> +       | comparable |
>> +       comparable := ((aString endsWith: '.mcz') and: [ aString size > 4 ])
>> +               ifTrue: [ aString allButLast: 4 ]
>> +               ifFalse: [ aString ].
>
> Thank you for the suggestion. Why not use aString asMCVersionName
> versionName here?

Copy-paste from parent.
When Chris introduced MCVersionName, he tried to use #asMCVersionName
there, but later he reverted it according to the history of the parent
method. It probably broke stuff (see below).

I think MCVersionName is something that should not exists in its current
form. Why?
- it's a subclass of ByteString, which limits the possible character set
- #= is not commutative when one object is an MCVersionName, the other is
a ByteString:
  'foo' = 'foo.mcz' asMCVersionName. "==> false"
  'foo.mcz' asMCVersionName = 'foo'. "==> true"
- the above property breaks the contract of #hash and #=, so they
sometimes misbehave in hashed collections

>
> Doesn't the repository include the version even if it is just a diffy
> version? You might say no because there is no complete snapshot, but

It does, but we can't look up the diffy version, because we can't know the
previous version the diff was made against.

> the diffy version has the same versionInfo after all, doesn't it?

Yes, it has the same versionInfo.


Levente

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Monticello-ul.726.mcz

Chris Muller-3
Hi Levente and Jakob,

> Am Mo., 29. Juni 2020 um 01:43 Uhr schrieb <[hidden email]>:
>>
>> + ----- Method: MCDirectoryRepository>>includesVersionNamed: (in category 'versions') -----
>> + includesVersionNamed: aString
>> +
>> +       | comparable |
>> +       comparable := ((aString endsWith: '.mcz') and: [ aString size > 4 ])
>> +               ifTrue: [ aString allButLast: 4 ]
>> +               ifFalse: [ aString ].
>
> Thank you for the suggestion. Why not use aString asMCVersionName
> versionName here?

Copy-paste from parent.
When Chris introduced MCVersionName, he tried to use #asMCVersionName
there, but later he reverted it according to the history of the parent
method. It probably broke stuff (see below). 

Hmm...  that's true, unfortunately, my comment only says, "Fix" it with no other details.  I can't help but wonder if it was due to a #class test somewhere..
 

I think MCVersionName is something that should not exists in its current
form. Why?
- it's a subclass of ByteString, which limits the possible character set

It's common in the IT industry for Name identifiers to be restricted to a subset of ASCII to support interoperation with as many types of systems as possible.  It seems an appropriate choice for MC Version names.
 
- #= is not commutative when one object is an MCVersionName, the other is
a ByteString:
        'foo' = 'foo.mcz' asMCVersionName. "==> false"
        'foo.mcz' asMCVersionName = 'foo'. "==> true"
- the above property breaks the contract of #hash and #=, so they
sometimes misbehave in hashed collections

Not in actual practice, though.  The intent was that all uses of version names would be ensuring they're dealing with the first-class MCVersionName.  If one, theoretically, were to mix Strings and MCVersionNames as in your example, yes, it would misbehave so... "don't do that."  :)

 - Chris


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Monticello-ul.726.mcz

Levente Uzonyi
On Tue, 30 Jun 2020, Chris Muller wrote:

> Hi Levente and Jakob,
>
>       > Am Mo., 29. Juni 2020 um 01:43 Uhr schrieb <[hidden email]>:
>       >>
>       >> + ----- Method: MCDirectoryRepository>>includesVersionNamed: (in category 'versions') -----
>       >> + includesVersionNamed: aString
>       >> +
>       >> +       | comparable |
>       >> +       comparable := ((aString endsWith: '.mcz') and: [ aString size > 4 ])
>       >> +               ifTrue: [ aString allButLast: 4 ]
>       >> +               ifFalse: [ aString ].
>       >
>       > Thank you for the suggestion. Why not use aString asMCVersionName
>       > versionName here?
>
>       Copy-paste from parent.
>       When Chris introduced MCVersionName, he tried to use #asMCVersionName
>       there, but later he reverted it according to the history of the parent
>       method. It probably broke stuff (see below). 
>
>
> Hmm...  that's true, unfortunately, my comment only says, "Fix" it with no other details.  I can't help but wonder if it was due to a #class test somewhere..
>  
>
>       I think MCVersionName is something that should not exists in its current
>       form. Why?
>       - it's a subclass of ByteString, which limits the possible character set
>
>
> It's common in the IT industry for Name identifiers to be restricted to a subset of ASCII to support interoperation with as many types of systems as possible.  It seems an appropriate choice for MC Version names.
>  
>       - #= is not commutative when one object is an MCVersionName, the other is
>       a ByteString:
>               'foo' = 'foo.mcz' asMCVersionName. "==> false"
>               'foo.mcz' asMCVersionName = 'foo'. "==> true"
>       - the above property breaks the contract of #hash and #=, so they
>       sometimes misbehave in hashed collections
>
>
> Not in actual practice, though.  The intent was that all uses of version names would be ensuring they're dealing with the first-class MCVersionName.  If one, theoretically, were to mix Strings and MCVersionNames as in your
> example, yes, it would misbehave so... "don't do that."  :)
If it shouldn't be mixed with strings, why is it a string?


Levente

>
>  - Chris
>
>