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!
Am Mo., 29. Juni 2020 um 01:43 Uhr schrieb commits@source.squeak.org:
- ----- 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?
On Mon, 29 Jun 2020, Jakob Reschke wrote:
Am Mo., 29. Juni 2020 um 01:43 Uhr schrieb commits@source.squeak.org:
- ----- 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
Hi Levente and Jakob,
Am Mo., 29. Juni 2020 um 01:43 Uhr schrieb commits@source.squeak.org:
- ----- 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
On Tue, 30 Jun 2020, Chris Muller wrote:
Hi Levente and Jakob,
> Am Mo., 29. Juni 2020 um 01:43 Uhr schrieb <commits@source.squeak.org>: >> >> + ----- 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 setIt'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
squeak-dev@lists.squeakfoundation.org
-
Chris Muller -
commits＠source.squeak.org -
Jakob Reschke -
Levente Uzonyi