[squeak-dev] I'd like to contribute to the JSON projectLevente Uzonyi leves at caesar.elte.huSun Nov 22 18:37:18 UTC 2020
Hi Tobias, On Sun, 22 Nov 2020, Tobias Pape wrote: > > >> On 22. Nov 2020, at 18:51, Levente Uzonyi <leves at caesar.elte.hu> wrote: >> >> Hi Tobias, >> >> On Sun, 22 Nov 2020, Tobias Pape wrote: >> >>> >>> >>>> On 22. Nov 2020, at 17:46, Levente Uzonyi <leves at caesar.elte.hu> wrote: >>>> Hi All, >>>> Since most (every?) practical use of #respondsTo: is to check whether it's safe to send the message or not, I think, contrary to what was mentioned in this thread, that #respondsTo: does not have to return true when sending the message would not result in an MNU. >>>> So, I suggest adding the following implementation to expose the dynamic nature of JsonObject: >>>> JsonObject >> #respondsTo: aSymbol >>>> >>>> | precedence | >>>> (super respondsTo: aSymbol) ifTrue: [ ^true ]. >>>> (precedence := aSymbol precedence) = 1 ifTrue: [ >>>> ^self includesKey: aSymbol ]. >>>> (precedence = 3 and: [ (aSymbol indexOf: $:) = aSymbol size ]) ifTrue: [ >>>> ^self includesKey: aSymbol allButLast ]. >>>> ^false >>> >>> That's nice! but why not make it simpler? >>> >>> JsonObject >> #respondsTo: aSymbol >>> >>> | precedence | >>> (super respondsTo: aSymbol) ifTrue: [ ^true ]. >>> aSymbol isSimpleGetter ifTrue: [^self includesKey: aSymbol]. >>> aSymbol isSimpleSetter ifTrue: [^self includesKey: aSymbol asSimpleGetter]. >>> ^false >> >> Three reasons: >> >> 1. performance >> >> | j s | >> Smalltalk garbageCollect. >> j := JsonObject new >> foo: 1; >> bar: 2; >> baz: 3; >> yourself. >> s := Symbol allSymbols. >> { >> [ s do: [ :each | ] ] bench. >> [ s do: [ :each | j respondsTo: each ] ] bench. >> [ s do: [ :each | j respondsTo2: each ] ] bench. "Your suggested implementation" >> } >> #( >> '1,630 per second. 613 microseconds per run. 0 % GC time.' >> '19 per second. 52.7 milliseconds per run. 0.09992 % GC time.' >> '1.18 per second. 850 milliseconds per run. 32.81709 % GC time.' >> ) >> >> Okay, that may not be too a realistic workload. The reason of the extreme >> slowdown and high GC time is rapid interning and GCing of Symbols >> created by #asSimpleGetter. >> >> If you change s to a handcrafted array that avoids Symbol creation, like >> >> s := #(yourself foo foo: bar bar: baz baz: foobar foobar: name name:) >> >> the numbers get better but still not as good as my suggestion: >> >> #( >> '4,970,000 per second. 201 nanoseconds per run. 38.02 % GC time.' >> '147,000 per second. 6.82 microseconds per run. 1.74 % GC time.' >> '92,300 per second. 10.8 microseconds per run. 1.09978 % GC time.') >> >> > > I thought you'd say that. > But "precedence" is one of the most obscure things around that part in the image. > > >> 2. backwards compatibility >> #isSimpleSetter and #isSimpleGetter are available since Squeak 5.3. I use this code in 5.1 and 5.2 images as well. > > Yea, Pre 5.3 I'd have said #asMutator. > >> >> >> 3. to use the same mechanism as #doesNotUnderstand: >> Have a look at that method. > > > Then I'd rather say change DNU too. > If you're down that hole (dnu/respondsTo) anyways, I don't buy the performance argument anymore. > > Not everything has to be as fast as possible. You seem to ignore that #doesNotUnderstand: is the most often used method of JsonObject. I assume you don't use the JSON package in production images, hence you don't care about performance. I do, so I'm not willing to change the implementation of #doesNotUnderstand: in my fork of the JSON package unless performance is at least as good as it is now. Levente > > Best regards > -Tobias > >> >> >> Levente >> >>> >>> -Tobias >>> >>>> Levente >>>> On Sun, 22 Nov 2020, Thiede, Christoph wrote: >>>>> (Depending on how this discussion will end, this reparented mcz file might be relevant to prevent further merging issues.) >>>>> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________ >>>>> Von: Thiede, Christoph >>>>> Gesendet: Montag, 16. November 2020 16:22:01 >>>>> An: squeak-dev >>>>> Betreff: AW: [squeak-dev] I'd like to contribute to the JSON project Hi Marcel, >>>>> so do you propose to remove the existing implementation of dynamic forwarding from JsonObject, too (or more precisely, pull it down into DynamicJsonObject)? If yes, I would worry about compatibility problems. If no, I do not >>>>> quite understand why one should override #doesNotUnderstand: but not #respondsTo: in a class. It seems a reasonable pattern for me to override them only together. :-) >>>>> Best, >>>>> Christoph >>>>> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________ >>>>> Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel >>>>> Gesendet: Donnerstag, 12. November 2020 10:07:28 >>>>> An: squeak-dev >>>>> Betreff: Re: [squeak-dev] I'd like to contribute to the JSON project Hi all. >>>>> I am in favor of adding JsonDynamicObject (or similar) which has those extra features. I would avoid putting that stuff into JsonObject. When parsing a JSON file, the dictionary class can be configured anyway. >>>>> Best, >>>>> Marcel >>>>> >>>>> Am 10.11.2020 10:16:50 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de>: >>>>> >>>>> Hi all, >>>>> >>>>> > And canUnderstand: ? Or is that being too picky? >>>>> >>>>> > If the doesNotUnderstand: is not visible externally then who cares? Isn't the contract (o respondsTo: m) ifFalse: [self should: [o m] raise: MessageNotUnderstood]], or respondsTo: not implies MNU ? >>>>> Well, my conception of the general contract would be exactly the following: >>>>> (o class canUnderstand: m) ifTrue: [ >>>>> self assert: [o respondsTo: m]]. >>>>> (o respondsTo: m) ifFalse: [ >>>>> self deny: [o class canUnderstand: m]]. >>>>> (o respondsTo: m) ifTrue: [ >>>>> self shouldnt: [o m] raise: MessageNotUnderstood]. >>>>> [o m] on: MessageNotUnderstood do: [ >>>>> self deny: [o respondsTo: m]]. >>>>> But I would *not* require the other direction of the implication - for #canUnderstand:, this is simply not possible for dynamic forwarding (unless we make false promises on the class side), and in my opinion, the >>>>> current discussion shows that the same argument applies for the second statement, too. >>>>> > I would like to keep the JSON library as simple as possible. Wer are just talking about syntactic sugar here, right? >>>>> IMHO, this goes beyond syntactic sugar. :-) As I tried to explain below, a proper implementation of #respondsTo: could be an essential prerequisite for using JsonObjects polymorphically with first-class object >>>>> instances. In my use case, this is a crucial feature and if my proposal is discarded, I will have to subclass JsonObject ... >>>>> Best, >>>>> Christoph >>>>> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________ >>>>> Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel >>>>> Gesendet: Dienstag, 10. November 2020 09:34:49 >>>>> An: squeak-dev >>>>> Betreff: Re: [squeak-dev] I'd like to contribute to the JSON project > and generate the getter setter on demand (via doesNotUnderstand:) >>>>> That's what I opted for, too, in: https://github.com/hpi-swa/MessageSendRecorder 's MessageSendRecordExtension. >>>>> Best. >>>>> Marcel >>>>> >>>>> Am 10.11.2020 09:32:07 schrieb Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>: >>>>> >>>>> Hi all, >>>>> for importing Matlab struct, I create classes on the fly and generate the getter setter on demand (via doesNotUnderstand:) >>>>> See MatFileReader package in http://www.squeaksource.com/STEM.html >>>>> Le mar. 10 nov. 2020 à 09:06, Marcel Taeumel <marcel.taeumel at hpi.de> a écrit : >>>>> > And canUnderstand: ? Or is that being too picky? >>>>> Ah, right. On the class level, it would be like Levente inferred from my suggestion. I only thought of #respondsTo: to answer "true" only for the simple setter/getters that have keys present in the actual >>>>> dictionary instance. Hmmm..... >>>>> I would like to keep the JSON library as simple as possible. Wer are just talking about syntactic sugar here, right? >>>>> Best, >>>>> Marcel >>>>> >>>>> Am 09.11.2020 21:08:14 schrieb Eliot Miranda <eliot.miranda at gmail.com>: >>>>> On Sun, Nov 8, 2020 at 11:04 PM Marcel Taeumel <marcel.taeumel at hpi.de> wrote: >>>>> Hi Levente. >>>>> Sounds right. If an object can answer to some extra messages via #doesNotUnderstand:, one should also override #respondsTo:. It is like #= and #hash. >>>>> And canUnderstand: ? Or is that being too picky? >>>>> I did not know about #dictionaryClass:. That's a powerful hook. >>>>> Best, >>>>> Marcel >>>>> >>>>> Am 09.11.2020 03:07:54 schrieb Levente Uzonyi <leves at caesar.elte.hu>: >>>>> >>>>> Hi Christoph, >>>>> >>>>> On Sun, 8 Nov 2020, Christoph Thiede wrote: >>>>> >>>>> > Hi Levente, >>>>> > >>>>> > would you mind to merge JSON-ct.41 (#respondsTo:) as well? This would be >>>>> > great because I depend on this functionality in another project and >>>>> > currently require your JSON fork in my baseline. :-) >>>>> >>>>> I cannot merge it because that would bring back long removed methods, and >>>>> MC wouldn't allow me to reject those. >>>>> But I can add the changes manually. >>>>> If I'm not mistaken, it's just a single method JsonObject >> #respondsTo:. >>>>> >>>>> What is the purpose of that method? >>>>> I'm asking because it has got no comment, so I'm not sure its >>>>> implementation is correct. >>>>> For example, should >>>>> >>>>> JsonObject new respondsTo: #foo: >>>>> >>>>> return false? >>>>> What should the following return? >>>>> >>>>> JsonObject new >>>>> foo: 1; >>>>> respondsTo: #foo: >>>>> >>>>> Another question is whether it is generally useful or not? >>>>> If it's not, you can still have the desired behavior by creating a >>>>> subclass. E.g.: >>>>> >>>>> JsonObject subclass: #PseudoObject >>>>> instanceVariableNames: '' >>>>> classVariableNames: '' >>>>> poolDictionaries: '' >>>>> category: 'PseudoObject' >>>>> >>>>> PseudoObject >> respondsTo: aSymbol >>>>> >>>>> ^ (super respondsTo: aSymbol) >>>>> or: [self includesKey: aSymbol] >>>>> >>>>> (Json new >>>>> dictionaryClass: PseudoObject; >>>>> readFrom: '{"foo": 42}' readStream) >>>>> respondsTo: #foo >>>>> "==> true" >>>>> >>>>> Levente >>>>> >>>>> > >>>>> > Best, >>>>> > Christoph >>>>> > >>>>> > >>>>> > >>>>> > -- >>>>> > Sent from: http://forum.world.st/Squeak-Dev-f45488.html >>>>> -- >>>>> _,,,^..^,,,_ >>>>> best, Eliot
More information about the Squeak-dev mailing list |