Ticket #2034 (new experimental)

Opened 4 years ago

Last modified 3 years ago

Experimental Select PMC

Reported by: cotto Owned by:
Priority: normal Milestone:
Component: core Version: 3.1.0
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description

tewk++ has contributed a Select PMC. While the interface solidifies, it should be considered experimental. This ticket exists to track its progress once it's been merged. (If src/pmc/select.pmc exists, it's been merged.)

Attachments

20110826.448bcca.master.gpp.txt.gz Download (5.7 KB) - added by jkeenan 3 years ago.
Merge of tewk/select branch causes all-g++ build to fail

Change History

in reply to: ↑ description ; follow-up: ↓ 2   Changed 4 years ago by plobsing

Replying to cotto:

tewk++ has contributed a Select PMC. While the interface solidifies, it should be considered experimental. This ticket exists to track its progress once it's been merged. (If src/pmc/select.pmc exists, it's been merged.)

Why is this being added as a core PMC? Can Parrot not operate without it? Is it expected to be used in the majority of programs? A large minority?

Every core PMC adds (albeit slightly) to the time required to start up parrot, the memory overhead consumed by parrot, and the number of long-lived GCables present. The question we've got to ask is, do we have a legitimate reason for taxing ALL Parrot users? or are we just too lazy to add a couple loadlib statements?

in reply to: ↑ 1   Changed 4 years ago by cotto

Replying to plobsing:

Replying to cotto:

tewk++ has contributed a Select PMC. While the interface solidifies, it should be considered experimental. This ticket exists to track its progress once it's been merged. (If src/pmc/select.pmc exists, it's been merged.)

Why is this being added as a core PMC? Can Parrot not operate without it? Is it expected to be used in the majority of programs? A large minority? Every core PMC adds (albeit slightly) to the time required to start up parrot, the memory overhead consumed by parrot, and the number of long-lived GCables present. The question we've got to ask is, do we have a legitimate reason for taxing ALL Parrot users? or are we just too lazy to add a couple loadlib statements?

I agree with this assessment. Thanks for switching the PMC to a dynpmc. I still would like to do another round of review on this PMC, but I won't complain if someone feels like merging it now.

in reply to: ↑ description   Changed 3 years ago by pmichaud

Bump.

Pm

  Changed 3 years ago by cotto

After looking at the Select dynpmc again, I'm not entirely impressed with the interface. It'll work, but something closer to Python's approach strikes me as better from a API design perspective.

I'm thinking of something like one or both of the following: (r_ready, w_ready, x_ready) = s.'select'(r, w, x, timeout)

s = new "Select" s.'register'(fh, "rwe") s.'register'(fh2, "re") s.'unregister'(fh3, "rwe") s.'modify'(fh4, "e") s.'select'(999)

if s.'error'() goto foo if s.'read'() goto bar

Thoughts are welcome. If this proposed interface looks good, I'll implement it. If not, I'm willing to implement an alternative suggestion. I do want to get some kind of select functionality into Parrot, but also feel like the current implementation in tewk/select has room for improvement.

  Changed 3 years ago by benabik

I created a short C file to test select:  https://gist.github.com/1171339

Basically, select.t fails on OS X because it returns both file handles as ready for error. We may just want to skip that test as being platform dependent...

follow-up: ↓ 8   Changed 3 years ago by jkeenan

In addition to not building on Windows and causing new test failures on Darwin, the merge of the tewk/select branch has now broken our build with g++. I will attach the output of 'make' run with g++ used for 'cc', 'link' and 'ld'.

By way of comparison, I did a checkout of the commit immediately preceding the tewk/select branch merge in the logs. I configured with all-g++ and was able to run 'make' and  make test without incident.

"Experimental" or not, this merge has caused so many problems that it needs to be reverted ASAP.

Thank you very much.

kid51

Changed 3 years ago by jkeenan

Merge of tewk/select branch causes all-g++ build to fail

  Changed 3 years ago by jkeenan

An all-g++ build on Darwin failed in much the same way as the all-g++ build on Linux posted earlier. Here's the tail of the make output:

/usr/bin/g++ -I./include -I./include/pmc -pipe -fno-common 
-Wno-long-double  -I/sw/include -I/opt/local/include -DHASATTRIBUTE_CONST 
 -DHASATTRIBUTE_DEPRECATED  -DHASATTRIBUTE_MALLOC  -DHASATTRIBUTE_NONNULL  
-DHASATTRIBUTE_NORETURN  -DHASATTRIBUTE_PURE  -DHASATTRIBUTE_UNUSED 
 -DHASATTRIBUTE_WARN_UNUSED_RESULT  -DHAS_GETTEXT -g      
-falign-functions=16 -funit-at-a-time -W -Wall -Waggregate-return 
-Wcast-align -Wcast-qual -Wchar-subscripts -Wcomment -Wdisabled-optimization 
-Wendif-labels -Wextra -Wformat -Wformat-extra-args -Wformat-nonliteral 
-Wformat-security -Wformat-y2k -Wimplicit -Wimport -Winit-self -Winline 
-Winvalid-pch -Wmissing-braces -Wmissing-field-initializers 
-Wno-missing-format-attribute -Wmissing-include-dirs -Wmultichar 
-Wpacked -Wparentheses -Wpointer-arith -Wreturn-type -Wsequence-point 
-Wsign-compare -Wstrict-aliasing -Wstrict-aliasing=2 -Wswitch 
-Wswitch-default -Wtrigraphs -Wundef -Wno-unused -Wunknown-pragmas 
-Wvariadic-macros -Wwrite-strings -fvisibility=hidden 
-Isrc/pmc -o src/pmc/filehandle.o -c src/pmc/filehandle.c
src/pmc/filehandle.c: 
  In function 'void Parrot_FileHandle_nci_setasync(parrot_interp_t*, PMC*)':
src/pmc/filehandle.c:1200: error: 
  'Parrot_io_async' was not declared in this scope
src/pmc/filehandle.c: 
  In function 'void Parrot_FileHandle_nci_setblocking(parrot_interp_t*, PMC*)':
src/pmc/filehandle.c:1228: error: 
  'Parrot_io_async' was not declared in this scope
make: *** [src/pmc/filehandle.o] Error 1

in reply to: ↑ 6   Changed 3 years ago by jkeenan

Replying to jkeenan:

In addition to not building on Windows and causing new test failures on Darwin, the merge of the tewk/select branch has now broken our build with g++. I will attach the output of 'make' run with g++ used for 'cc', 'link' and 'ld'.

NotFound++ fixed the g++ builds with 30a829d. (See, e.g.,  Smolder 22061 (Linux) or  Smolder 22063 (Darwin). But we continue to have the problem of failures in t/dynpmc/select.t on OSes other than Linux; see the latter Smolder.

Thank you very much.

kid51

  Changed 3 years ago by benabik

As far as I can tell t/dynpmc/select.t is completely bogus. It uses select on file handles which behaves differently on different platforms and doesn't work at all on Windows. Depending on what manage you read, it's a completely meaningless operation. It should probably be TODO'd until replaced by a socket-based test.

  Changed 3 years ago by jkeenan

The failing test in t/dynpmc/select.t was not fixed, replaced, deleted, or TODOed for Darwin in the 3.8 release. In addition, Kevin Polulak (++ serving as RM for first time) reported it was failing on Open Solaris as well.

Note: See TracTickets for help on using tickets.