[squeak-dev] I'd like to contribute to the JSON project
Levente Uzonyi
leves at caesar.elte.hu
Sun 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
|