[Webkit-unassigned] [Bug 231101] [WinCairo] Support run-jsc-stress-tests without posix commands

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 6 09:12:21 PDT 2021


--- Comment #9 from Angelos Oikonomopoulos <angelos at igalia.com> ---
(In reply to Adrian Perez from comment #6)
> Comment on attachment 440092 [details]
> Patch
> Hello, and thanks for the patch. I think this is going in the right
> direction, but I have a couple of suggestions to make it even better.
> Please read on below :)

Thanks for taking a look Adrian, IMHO you're more right than you think :-)

> View in context:
> https://bugs.webkit.org/attachment.cgi?id=440092&action=review
> > Tools/Scripts/run-jsc-stress-tests:2334
> > +    def each_line
> I suppose this function could be just a function; there is no
> need to put it inside a class, right?

The reason I suggested a class is so that it would be interchangeable with the IO object produced by IO.popen(find_cmd). Technically you could wrap /that/ in a function so as to have a common interface. I thought it was more idiomatic that way. Opinions may differ.

> > Tools/Scripts/run-jsc-stress-tests:2367
> > +            statusFileEnumerator.each_line {
> I would completely remove the usage of the “find” command and use the
> same code that uses Ruby functions to locate and read the result files.
> The advantages would be having only one code path to maintain that works
> for all ports, and avoiding spawning a bunch of subprocesses here.

The reason I didn't suggest that is I introduced find_cmd locally so as to share the local code with the one dealing with remote boxes. This way, changes to this part of the code would automatically apply to the "remote" code as well. I didn't realize when replying, but this implicitly prioritizes the find-based remote code path over the ruby-based non-posix code path.

However, I think the choice here is clear: the find code should be much slower (because of all those sh invocations), so we should go with the ruby version for the local case (though measuring would be nice at some point).

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211006/91c5d6c2/attachment.htm>

More information about the webkit-unassigned mailing list