An issue with Collections-ul.667
Locked
An issue with Collections-ul.667
 
|
Hi Levente, I just wanted to note a potential problem with Collections-ul.667 Name: Collections-ul.667 Author: ul Time: 10 October 2015, 3:40:46.732 pm UUID: 7cd785ff-1839-4075-ac56-4b71054529d8 Ancestors: Collections-topa.666 Added the missing methods of ByteArray's platform independent access category: #signedByteAt: #signedByteAt:put: #long64At:put:bigEndian: and #long64At:bigEndian:. The 64-bit methods are not optimized. In this version you provided ByteArray>>#signedByteAt:[put:] which conflicts with FFI-Kernel's methods of the same name. There's a difference in semantics. If you look at ExternalStructure's class comment it points out that "This class provides an abstract base for all structures that can be used by external functions. ExternalStructures have two possible handle types: - ExternalAddress If the handle is an external address then the object described does not reside in the Smalltalk object memory. - ByteArray If the handle is a byte array then the object described resides in Smalltalk memory. Useful methods should be implemented by subclasses of ExternalStructure using the common ByteArray/ExternalAddress platform dependent access protocol which will transparently access the correct memory location." i.e. the primitives used automatically differentiate between an ExternalAddress holding a pointer to external memory, in which case the datum is accessed via that location, or a ByteArray, in which case the datum is accessed in the ByteArray. Your versions only do the latter and would hence misinterpret data in ExternalAdderss instances. Here's the FFI code: ByteArray>>signedByteAt: byteOffset "Return a 8bit signed integer starting at the given byte offset" ^self integerAt: byteOffset size: 1 signed: true ByteArray>>integerAt: byteOffset size: nBytes signed: aBoolean "Primitive. Return an integer of nBytes size from the receiver. Note: This primitive will access memory in the outer space if invoked from ExternalAddress." <primitive: 'primitiveFFIIntegerAt' module:'SqueakFFIPrims'> ^self primitiveFailed And here's yours: ByteArray>>signedByteAt: index "Return an 8-bit signed integer quantity from the given byte index." | byte | (byte := self at: index) <= 16r7F ifTrue: [ ^byte ]. ^byte - 16r100 Now of course this isn't an issue in builds that load the FFI after Collections, but it does affect images in which FFI-Kernel was loaded and then the image was updated to instal Collections-ul.667. I'm not sure what the right fix is, but I thought I'd better mention it. One obvious one is to implement the FFI methods in ExternalAddress>>#signedByteAt:[put:], and if folks agree I can do this. Let me know. Pharoers, if I do this, you'll need to make sure that the Pharo base includes ByteArray>>#signedByteAt:[put:]. _,,,^..^,,,_ best, Eliot |
|
|
That sounds like a decent compromise. I would strongly request that a ByteArray>>signedByteAt:(put:) exist in the base image. The lack was why it was added - and it is useful for add-on packages such as my Kafka one. Thanks, CNBC Sent from my iPhone
|
Re: An issue with Collections-ul.667
|
|
In reply to this post by Eliot Miranda-2
Hi Eliot,
If we want to keep the methods in the image, then I see two possibilities: - One of them is, as you suggested, to add the methods to ExternalAddress. This is a lot cleaner than relying on inheritance, and would also avoid the updating issue you mentioned. It's fully backwards-compatible as well - with these changes FFI can still be used from older images. - Another option would be to move the primitive from the FFI plugin to the VM. I didn't check how the primitive works, but I assume that it uses the external objects array to get known of ExternalAddress, so I think this is also possible. This way we could use the primitive from the image side code as well, which should be faster especially for larger numbers. Btw, there are other semantic differences between the two implementations. The primitive is always strict about the accepted range of input values, but the image side code is more lenient about under- and overflows, which allows some tricks. E.g.: (ByteArray new: 1) signedByteAt: 1 put: 255; signedByteAt: 1 returns -1 using the code in Trunk, but it will fail with the primitive. Levente On Tue, 16 Feb 2016, Eliot Miranda wrote: > Hi Levente, > > I just wanted to note a potential problem with Collections-ul.667 > > Name: Collections-ul.667 > Author: ul > Time: 10 October 2015, 3:40:46.732 pm > UUID: 7cd785ff-1839-4075-ac56-4b71054529d8 > Ancestors: Collections-topa.666 > > Added the missing methods of ByteArray's platform independent access category: #signedByteAt: #signedByteAt:put: #long64At:put:bigEndian: and #long64At:bigEndian:. The 64-bit methods are not > optimized. > > > In this version you provided ByteArray>>#signedByteAt:[put:] which conflicts with FFI-Kernel's methods of the same name. There's a difference in semantics. If you look at > ExternalStructure's class comment it points out that > > "This class provides an abstract base for all structures that can be used by external functions. ExternalStructures have two possible handle types: > - ExternalAddress > If the handle is an external address then the object described does not reside in the Smalltalk object memory. > - ByteArray > If the handle is a byte array then the object described resides in Smalltalk memory. > Useful methods should be implemented by subclasses of ExternalStructure using the common ByteArray/ExternalAddress platform dependent access protocol which will transparently access the > correct memory location." > > i.e. the primitives used automatically differentiate between an ExternalAddress holding a pointer to external memory, in which case the datum is accessed via that location, or a ByteArray, > in which case the datum is accessed in the ByteArray. Your versions only do the latter and would hence misinterpret data in ExternalAdderss instances. Here's the FFI code: > > ByteArray>>signedByteAt: byteOffset > "Return a 8bit signed integer starting at the given byte offset" > ^self integerAt: byteOffset size: 1 signed: true > > ByteArray>>integerAt: byteOffset size: nBytes signed: aBoolean > "Primitive. Return an integer of nBytes size from the receiver. > Note: This primitive will access memory in the outer space if > invoked from ExternalAddress." > <primitive: 'primitiveFFIIntegerAt' module:'SqueakFFIPrims'> > ^self primitiveFailed > > And here's yours: > ByteArray>>signedByteAt: index > "Return an 8-bit signed integer quantity from the given byte index." > | byte | > (byte := self at: index) <= 16r7F ifTrue: [ ^byte ]. > ^byte - 16r100 > > Now of course this isn't an issue in builds that load the FFI after Collections, but it does affect images in which FFI-Kernel was loaded and then the image was updated to > instal Collections-ul.667. > > I'm not sure what the right fix is, but I thought I'd better mention it. One obvious one is to implement the FFI methods in ExternalAddress>>#signedByteAt:[put:], and if folks agree I can > do this. Let me know. Pharoers, if I do this, you'll need to make sure that the Pharo base includes ByteArray>>#signedByteAt:[put:]. > > _,,,^..^,,,_ > best, Eliot > > |
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |