Vm-dev October 2020

vm-dev@lists.squeakfoundation.org

26 participants
351 discussions

Re: [Vm-dev] [OpenSmalltalk/opensmalltalk-vm] Unix(macOS) SocketPlugin: (7deac02)
by smalltalking 10 Oct '20

10 Oct '20

Hi Eliot, it's me Levente. My comment was about the comparison of select's return value with 1 instead of checking whether it's greater than 0. If there is a file descriptor available for write in the set, the returned value always will be 1 since there's only one file descriptor in the set. So, == 1 is the same as > 0. The first argument of select is ignored on windows: https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-sel… But on other systems (e.g. in sqUnixSocket.c), it should be set to > the highest-numbered file descriptor in any of the three sets, plus 1. The indicated file descriptors in each set are checked, up to this limit (but see BUGS). See https://www.man7.org/linux/man-pages/man2/select.2.html. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/commit/7deac028aeeb769c87…

1 0

Re: [Vm-dev] [OpenSmalltalk/opensmalltalk-vm] Unix(macOS) SocketPlugin: (7deac02)
by Eliot Miranda 10 Oct '20

10 Oct '20

Hi shaping. That’s a misreading. The first argument is the number of file descriptors in the various fds arguments: The nfds argument specifies the range of descriptors to be tested. The first nfds descriptors shall be checked in each set; that is, the descriptors from zero through nfds-1 in the descriptor sets shall be examined. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/commit/7deac028aeeb769c87…

1 0

Re: [Vm-dev] [OpenSmalltalk/opensmalltalk-vm] Unix(macOS) SocketPlugin: (7deac02)
by smalltalking 10 Oct '20

10 Oct '20

The set has exactly one element, so this change is unnecessary. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/commit/7deac028aeeb769c87…

1 0

Re: [Vm-dev] [OpenSmalltalk/opensmalltalk-vm] Unix(macOS) SocketPlugin: (7deac02)
by smalltalking 10 Oct '20

10 Oct '20

The first parameter of select should be the largest file descriptor in the set + 1, so s + 1 in this case. The original code had s + 1. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/commit/7deac028aeeb769c87…

1 0

Re: [Vm-dev] [OpenSmalltalk/opensmalltalk-vm] Unix(macOS) SocketPlugin: (7deac02)
by Eliot Miranda 10 Oct '20

10 Oct '20

Hi Holger, first I don’t do anything. This code is historical. I’m simply trying to understand it and fix it. Second, the reason it’s 1 is that it’s testing for writability of a single file descriptor which happens to be that of a socket. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/commit/7deac028aeeb769c87…

1 0

Re: [Vm-dev] [OpenSmalltalk/opensmalltalk-vm] Unix(macOS) SocketPlugin: (7deac02)
by Holger Freyther 09 Oct '20

09 Oct '20

It's late but why do you hardcode the first parameter of select to 1? While I see your fd seems to be higher than 0? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/commit/7deac028aeeb769c87…

1 0

Re: [Vm-dev] [OpenSmalltalk/opensmalltalk-vm] Unix(macOS) SocketPlugin: (7deac02)
by Tobias Pape 09 Oct '20

09 Oct '20

(I looked into it per your request :)) I figured that ditching `aioPoll` might not work; that said, I am not deep enough in this to understand why a writable socket might return 0. My brain start putting up "race condition?!?" warning signs, so there's that, but, ultimately, thats where my knowledge ends here -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/commit/7deac028aeeb769c87…

1 0

Installing high dpi support and backwards compatibility of the VM
by Eliot Miranda 09 Oct '20

09 Oct '20

Hi All, ideally adding the high dpi support to the VM will not break backwards-compatibility. But that implies that the VM is informed before it creates the display of whether to create a high dpi display or not. Off-list Tobias asked me where the VM sets up the display on Mac and I was surprised by the answer. I thought it would be as part of beDisplay. But it's actually as a side-effect of DisplayScreen class>>actualScreenSize, primitive 106, which calls the ioScreenSize function. It is this functions' responsibility to actually create the display, deriving the size from the savedWindowSize info in the image header (which can or could be overridden on the VM command line, and is when -headless is supplied). So any new primitive added to allow DisplayScreen to inform the VM of whether to use high dpi or not would have to be invoked before primitive 106. So one way to implement this is to modify the chain of invocations leading up to primitive 106. For this route I'd like to propose the following refactoring: DisplayScreen class>>actualScreenSize <primitive: 106> ^ 640@480 becomes DisplayScreen class>>actualScreenSize self primitiveUseHighDPI: self useHighDPI. "where this is a preference" ^self primitiveScreenSize primitiveScreenSize <primitive: 106> ^ 640@480 Another route is to make the useHighDPI flag part of the image header state alongside the saved window size. This would mean it was added to the flags accessed via vmParameterAt: 48. There could be a command-line argument to override. Finally I note that the beDisplay primitive simply stores the display object in the specialObjectsArray and assigns the interpreter variables that track the display, displayBits, displayWidth, displayHeight & displayDepth. It then invokes ioNoteDisplayChangedwidthheightdepth, but *all* the implementations of this function are empty. I propose that we should eliminate 5this call and its implementation. It is confusing to follow it and find it does nothing. The argument could be that a platform might require it. But if that's so we can always put it back. We have an existence proof in all our platforms that this is unlikely. Thoughts? _,,,^..^,,,_ best, Eliot

2 3

Re: [Vm-dev] [squeak-dev] Installing high dpi support and backwards compatibility of the VM
by Eliot Miranda 09 Oct '20

09 Oct '20

Hi Tobias, On Thu, Oct 8, 2020 at 11:57 PM Tobias Pape <Das.Linux(a)gmx.de> wrote: > Hi > > > On 08.10.2020, at 19:58, Eliot Miranda <eliot.miranda(a)gmail.com> wrote: > > > > Hi All, > > > > ideally adding the high dpi support to the VM will not break > backwards-compatibility. But that implies that the VM is informed before > it creates the display of whether to create a high dpi display or not. > Off-list Tobias asked me where the VM sets up the display on Mac and I was > surprised by the answer. > > > > I thought it would be as part of beDisplay. But it's actually as a > side-effect of DisplayScreen class>>actualScreenSize, primitive 106, which > calls the ioScreenSize function. It is this functions' responsibility to > actually create the display, deriving the size from the savedWindowSize > info in the image header (which can or could be overridden on the VM > command line, and is when -headless is supplied). > > > > So any new primitive added to allow DisplayScreen to inform the VM of > whether to use high dpi or not would have to be invoked before primitive > 106. So one way to implement this is to modify the chain of invocations > leading up to primitive 106. For this route I'd like to propose the > following refactoring: > > > > DisplayScreen class>>actualScreenSize > > <primitive: 106> > > ^ 640@480 > > > > becomes > > > > DisplayScreen class>>actualScreenSize > > self primitiveUseHighDPI: self useHighDPI. "where this is a > preference" > > ^self primitiveScreenSize > > > > primitiveScreenSize > > <primitive: 106> > > ^ 640@480 > > > > > Here's another idea: > We already have > > DisplayScreen class>>actualScreenScaleFactor > <primitive: 'primitiveScreenScaleFactor'> > ^ 1.0 > > And if we change DisplayScreen class>>startUp to > > DisplayScreen class>>startUp "DisplayScreen startUp" > Display setScaleFactor: self actualScreenScaleFactor. > Display setExtent: self actualScreenSize depth: Display > nativeDepth. > Display beDisplay > Very nice. Let's go with this. > > Then the contract could be: > > "Iff you call primitiveScreenScaleFactor before any call to primitive 106, > then you opt in to possibly high dpi" > > That way, we do not have to change any image at all, cause older images > just don't call that primitive. > Yep, works for me. > > > > > > Another route is to make the useHighDPI flag part of the image header > state alongside the saved window size. This would mean it was added to the > flags accessed via vmParameterAt: 48. There could be a command-line > argument to override. > > Maybe a cmd-line parameter in any case… > +1 > > > > > > > Finally I note that the beDisplay primitive simply stores the display > object in the specialObjectsArray and assigns the interpreter variables > that track the display, displayBits, displayWidth, displayHeight & > displayDepth. It then invokes ioNoteDisplayChangedwidthheightdepth, but > *all* the implementations of this function are empty. I propose that we > should eliminate 5this call and its implementation. It is confusing to > follow it and find it does nothing. The argument could be that a platform > might require it. But if that's so we can always put it back. We have an > existence proof in all our platforms that this is unlikely. Thoughts? > > > Funny. The mac vm says "/* This is invoked when the GC moves the display > bitmap. For now do nothing. */" Does the GC ever do that actually? > Not now. It used to. One would see the pixels in the display become nonsense noise as the GC moved the display underneath the screen refresh. But now in Spur the beDisplay primitive pins the display bits. > It's also fairly recent: > > 78c402ea71ebcc9db12496f81021fdb9b57deb5f (Fri May 12 19:29:45 2017) > > StackInterpreter: > Simplify and make robust display bitmap access for display update. > The old code > required platforms that needed to redraw at arbitrary times to have to > access > the display bits through interpreterProxy->displayObject, decoding it > each time. > There exists a small window during compaction, etc, during whiuch such > access > will fail and cause a VM crash. The new code provides four variables > to > reference the display, displayBits, displayWidth, displayHeight and > displayDepth, which are assigned appropriately in the > primitiveBeDisplay > primitive. After a GC the interpreter checks if the displayBits have > changed > location and if so calls ioNoteDisplayChanged:width:height:depth: > (ioNoteDisplayChangedwidthheightdepth) to inform the platform of the > change > (currently all platforms implement this as a null function). > > > So, old (<2017) code cannot depend on it, new code does not. If the GC > issue is moot, we can ditch it. > Even if the GC does move the display, ioNoteDisplayChangedwidthheightdepth can't be called until after GC, which means that from the time the GC moves the display to the time the GC finishes, the display image is corrupted. That's fixed in Spur but in V3 you'll see that happen, especially if you resize the display (which allocates a new bitmap). At least on Mac I would see it regularly. > > > Best regards > -Tobias > > > > > > -- _,,,^..^,,,_ best, Eliot

1 0

Re: [Vm-dev] [OpenSmalltalk/opensmalltalk-vm] Unix(macOS) SocketPlugin: (7deac02)
by Eliot Miranda 09 Oct '20

09 Oct '20

Tobias, you need to read the code to see why we can't ditch aioPoll. Perhaps we could ditch the select call in socketWritable, but we would need to test on all platforms. However, here's how you can see that select does indeed answer 0 for a writable socket on macOS (I'm using 10.13.6). 1. compile a debug VM using the version immediately before this commit, making sure the build supplies -DAIO_DEBUG. 2. run with logging turned on, e.g. $ ../build.macos64x64/squeak.cog.spur/SqueakDebug.app/Contents/MacOS/Squeak -aiolog trunk6-64.image 3. run the following in a workspace ``` | tcpSend tcpRecv sendProc recvProc | [ | address port opt send1 recv2 sent | address := NetNameResolver addressForName: '127.0.0.1' timeout: 20. port := 31259. tcpSend := Socket newTCP. tcpSend setOption: 'SO_REUSEADDR' value: 1. self assert: 0 = tcpSend socketError description: 'Error occured while setting SO_REUSEADDR'. opt := tcpSend getOption: 'SO_REUSEADDR'. self assert: opt first isZero & opt last isZero not description: 'SO_REUSEADDR couldn''t be set'. tcpSend setOption: 'SO_REUSEPORT' value: 1. self assert: 0 = tcpSend socketError description: 'Error occured while setting SO_REUSEPORT'. opt := tcpSend getOption: 'SO_REUSEPORT'. self assert: opt first isZero & opt last isZero not description: 'SO_REUSEPORT couldn''t be set'. "tcpSend setOption: 'TCP_NODELAY' value: 1." send1 := UUID new. tcpRecv := Socket newTCP. tcpRecv setOption: 'SO_REUSEADDR' value: 1. self assert: 0 = tcpRecv socketError. tcpRecv setOption: 'SO_REUSEPORT' value: 1. self assert: 0 = tcpRecv socketError. tcpRecv setPort: port. self assert: port = tcpRecv localPort. recv2 := UUID new. recvProc := [| received | received := 0. tcpRecv waitForConnectionFor: 200000. [received < 16] whileTrue: [received := received + (tcpRecv receiveDataInto: recv2 startingAt: received + 1) "No need to yield here, because #receiveDataInto:startingAt: will either wait on the readSemaphore of the socket or signal an error." ] ] newProcess. sendProc := [tcpSend connectTo: address port: port. sent := tcpSend sendData: send1] newProcess. recvProc resume. sendProc resume. (Delay forMilliseconds: 200) wait. self assert: sendProc isTerminated description: 'sendProc hasn''t terminated till the deadline'; assert: recvProc isTerminated description: 'recvProc hasn''t terminated till the deadline'; assert: 16 = sent description: ('{1} bytes were sent instead of 16' format: { sent }); assert: send1 = recv2 description: 'sent and received bytes differ'] ensure: [tcpSend ifNotNil: [ tcpSend destroy ]. tcpRecv ifNotNil: [ tcpRecv destroy ]. sendProc ifNotNil: [ sendProc terminate ]. recvProc ifNotNil: [ recvProc terminate ]]``` 4. notice how in the log you see ``` 32032 0 sqUnixSocket.c:1141 sqSocketSendDone(16) !socketWritable 32032 0 aio.c:521 aioHandle(16, dataHandler, 5/AIO_WX) 32032 0 sqUnixSocket.c:749 socketStatus 16: Connected O_ASYNC|O_NONBLOCK 32032 0 sqUnixSocket.c:480 dataHandler(16=16, 0x7ff901521f30, 4/AIO_W) 32032 0 sqUnixSocket.c:499 notify 16 write 32032 0 sqUnixSocket.c:1141 sqSocketSendDone(16) !socketWritable 32032 0 aio.c:521 aioHandle(16, dataHandler, 5/AIO_WX) 32032 0 sqUnixSocket.c:749 socketStatus 16: Connected O_ASYNC|O_NONBLOCK 32032 0 sqUnixSocket.c:480 dataHandler(16=16, 0x7ff901521f30, 4/AIO_W) 32032 0 sqUnixSocket.c:499 notify 16 write 32032 0 sqUnixSocket.c:1141 sqSocketSendDone(16) !socketWritable 32032 0 aio.c:521 aioHandle(16, dataHandler, 5/AIO_WX) 32032 0 sqUnixSocket.c:749 socketStatus 16: Connected O_ASYNC|O_NONBLOCK 32032 0 sqUnixSocket.c:480 dataHandler(16=16, 0x7ff901521f30, 4/AIO_W) 32032 0 sqUnixSocket.c:499 notify 16 write 32032 0 sqUnixSocket.c:1141 sqSocketSendDone(16) !socketWritable 32032 0 aio.c:521 aioHandle(16, dataHandler, 5/AIO_WX)...``` and each line that says ```sqUnixSocket.c:1141 sqSocketSendDone(16) !socketWritable``` is proof that select is returning 0. It comes form this: ```static int socketWritable(int s) { struct timeval tv = { 0, 0 }; // i.e. poll fd_set fds; FD_ZERO(&fds); FD_SET(s, &fds); #ifdef AIO_DEBUG { int r = select(1, 0, &fds, 0, &tv); if (r < 0) perror("socketWritable: select(1,0,&fd,0,&poll)"); return r > 0; } #else return select(1, 0, &fds, 0, &tv) > 0; #endif }``` i.e. ```r > 0``` is always false, and since there is no output from the perror we can conclude that select must be answering 0. Now compile a VM from this commit and see that the example now works and that the log looks healthy: ``` 413959 0 sqUnixSocket.c:788 listenOnPortBacklogSize(18, 1) 413959 0 aio.c:452 aioEnable(18): Elicit SIGIO via O_ASYNC/fcntl 413959 0 aio.c:521 aioHandle(18, acceptHandler, 3/AIO_RX) 413959 0 sqUnixSocket.c:757 socketStatus 18: WaitingForConnection O_ASYNC|O_NONBLOCK 413959 0 sqUnixSocket.c:757 socketStatus 18: WaitingForConnection O_ASYNC|O_NONBLOCK 413959 0 sqUnixSocket.c:757 socketStatus 16: Unconnected !fcntl(fd,F_GETFL,0) !! 413959 0 sqUnixSocket.c:2226 connectToAddressSize(16) 413959 0 aio.c:452 aioEnable(16): Elicit SIGIO via O_ASYNC/fcntl 413959 0 sqUnixSocket.c:2245 connect() => -1 413959 0 aio.c:521 aioHandle(16, connectHandler, 5/AIO_WX) 413961 2 sqUnixSocket.c:757 socketStatus 16: WaitingForConnection O_ASYNC|O_NONBLOCK 413961 0 sqUnixSocket.c:445 connectHandler(16, 0x7fd0b34e2b90, 4) 413961 0 sqUnixSocket.c:470 notify 16 conn 413961 0 sqUnixSocket.c:388 acceptHandler(18, 0x7fd0b34e2ca0 ,2) 413961 0 aio.c:565 aioDisable(18) 413961 0 aio.c:545 aioSuspend(18) 413961 0 aio.c:452 aioEnable(19): Elicit SIGIO via O_ASYNC/fcntl 413961 0 sqUnixSocket.c:435 notify 19 conn 413961 0 sqUnixSocket.c:757 socketStatus 19: Connected O_ASYNC|O_NONBLOCK 413961 0 aio.c:521 aioHandle(19, dataHandler, 3/AIO_RX) 413961 0 sqUnixSocket.c:1134 receiveDataAvailable(19) -> false [aioHandle is set] 413961 0 sqUnixSocket.c:757 socketStatus 19: Connected O_ASYNC|O_NONBLOCK 413961 0 sqUnixSocket.c:757 socketStatus 16: Connected O_ASYNC|O_NONBLOCK 413961 0 sqUnixSocket.c:1151 sqSocketSendDone(16) !socketWritable 413961 0 aio.c:521 aioHandle(16, dataHandler, 5/AIO_WX) 413961 0 sqUnixSocket.c:757 socketStatus 16: Connected O_ASYNC|O_NONBLOCK 413961 0 sqUnixSocket.c:481 dataHandler(16=16, 0x7fd0b34e2b90, 4/AIO_W) 413961 0 sqUnixSocket.c:501 notify 16 write 413961 0 sqUnixSocket.c:1242 TCP sendData(16, 16) 413961 0 sqUnixSocket.c:1262 sendData(16) done = 16 413961 0 sqUnixSocket.c:481 dataHandler(19=19, 0x7fd0b34e2ca0, 2/AIO_R) 413961 0 sqUnixSocket.c:498 notify 19 read 413961 0 sqUnixSocket.c:1120 receiveDataAvailable(19) -> true 413961 0 sqUnixSocket.c:1208 receiveData(19) done = 16 414163 202 sqUnixSocket.c:757 socketStatus 16: Connected O_ASYNC|O_NONBLOCK 414163 0 sqUnixSocket.c:1000 destroy(16) 414163 0 sqUnixSocket.c:983 abortConnection(16) 414163 0 sqUnixSocket.c:945 closeConnection(16) 414163 0 aio.c:565 aioDisable(16) 414163 0 aio.c:545 aioSuspend(16) 414163 0 sqUnixSocket.c:965 closeConnection: disconnected 414163 0 sqUnixSocket.c:757 socketStatus 19: Connected O_ASYNC|O_NONBLOCK 414163 0 sqUnixSocket.c:1000 destroy(19) 414163 0 sqUnixSocket.c:983 abortConnection(19) 414163 0 sqUnixSocket.c:945 closeConnection(19) 414163 0 aio.c:565 aioDisable(19) 414163 0 aio.c:545 aioSuspend(19) 414163 0 sqUnixSocket.c:965 closeConnection: disconnected``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/commit/7deac028aeeb769c87…

1 0

← Newer
1
...
27
28
29
30
31
32
33
...
36
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

Vm-dev October 2020