task -> thread #8522

ajnirp · 2015-11-13T21:46:21Z

yay for Python's os.walk

highfive · 2015-11-13T21:46:28Z

Warning

These commits modify unsafe code. Please review it carefully!
These commits modify layout code, but no reftests are modified. Please consider adding a reftest!

Manishearth · 2015-11-14T09:57:34Z

Reviewed 42 of 149 files at r1.
Review status: 31 of 125 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

^{ORGANIZATION.md, line 74 [r1] (raw file):}
Don't think this should have been renamed. Not sure/.

^{ports/cef/interfaces/cef_render_process_handler.rs, line 116 [r1] (raw file):}
I think "task" is CEF language, so it shouldn't be replaced. ditto for everything else in cef/x

Comments from the review on Reviewable.io

ajnirp · 2015-11-14T13:09:24Z

Review status: 31 of 125 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

^{ORGANIZATION.md, line 74 [r1] (raw file):}
i'm awaiting confirmation to roll back the renaming

^{ports/cef/interfaces/cef_render_process_handler.rs, line 116 [r1] (raw file):}
reverted

Comments from the review on Reviewable.io

jdm · 2015-11-14T13:51:30Z

Yeah, rust-task_info refers to Mach tasks (ie. the OS X kernel): https://github.com/wenderen/servo/blob/8512-rename-task-to-thread/support/rust-thread_info/src/task_info.c

ajnirp · 2015-11-20T12:29:44Z

(ping)

Manishearth · 2015-11-24T12:41:02Z

Reviewed 106 of 149 files at r1.
Review status: 109 of 115 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

components/profile/Cargo.toml, line 32 [r1] (raw file):
did we rename this dep?xc

Comments from the review on Reviewable.io

Manishearth · 2015-11-24T12:49:03Z

Reviewed 4 of 13 files at r2, 10 of 10 files at r3.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

Comments from the review on Reviewable.io

ajnirp · 2015-11-24T14:40:06Z

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

components/profile/Cargo.toml, line 32 [r1] (raw file):
oops

Comments from the review on Reviewable.io

jdm · 2015-11-25T19:22:13Z

components/util/workqueue.rs

@@ -300,7 +300,7 @@ impl<QueueData: Sync, WorkData: Send> WorkQueue<QueueData, WorkData> {
        self.work_count += 1
    }

-    /// Synchronously runs all the enqueued tasks and waits for them to complete.
+    /// Synchronously runs all the enqueued threads and waits for them to complete.


This should be reverted.

jdm · 2015-11-25T19:23:28Z

ports/cef/interfaces/cef_task.rs

@@ -47,15 +47,15 @@ use std::mem;
 use std::ptr;

 //
-// Implement this structure for asynchronous task execution. If the task is
+// Implement this structure for asynchronous thread execution. If the thread is


The changes to this file will need to be reverted, since it's a CEF API.

jdm · 2015-11-25T19:25:52Z

components/script/dom/websocket.rs

@@ -466,13 +466,13 @@ impl WebSocketMethods for WebSocket {
 }


-/// Task queued when *the WebSocket connection is established*.
-struct ConnectionEstablishedTask {
+/// Thread queued when *the WebSocket connection is established*.


The changes for this type and comment should be reverted.

jdm · 2015-11-25T19:26:03Z

components/script/dom/websocket.rs

@@ -496,11 +496,11 @@ impl Runnable for ConnectionEstablishedTask {
    }
 }

-struct BufferedAmountTask {
+struct BufferedAmountThread {


Please revert this change.

jdm · 2015-11-25T19:26:11Z

components/script/dom/websocket.rs

@@ -514,11 +514,11 @@ impl Runnable for BufferedAmountTask {
    }
 }

-struct CloseTask {
+struct CloseThread {


Please revert this.

jdm · 2015-11-25T19:26:20Z

components/script/dom/websocket.rs

@@ -551,16 +551,16 @@ impl Runnable for CloseTask {
    }
 }

-struct MessageReceivedTask {
+struct MessageReceivedThread {


Please revert this.

jdm · 2015-11-25T19:30:49Z

components/script/dom/filereader.rs

 fn perform_annotated_read_operation(gen_id: GenerationId, data: ReadMetaData, blob_contents: Receiver<Vec<u8>>,
    filereader: TrustedFileReader, script_chan: Box<ScriptChan + Send>) {
    let chan = &script_chan;
    // Step 4
-    let task = box FileReaderEvent::ProcessRead(filereader.clone(), gen_id);
-    chan.send(CommonScriptMsg::RunnableMsg(FileRead, task)).unwrap();
+    let thread = box FileReaderEvent::ProcessRead(filereader.clone(), gen_id);


This rename should be reverted.

jdm · 2015-11-25T19:30:59Z

components/script/dom/filereader.rs

@@ -404,27 +404,27 @@ impl Runnable for FileReaderEvent {
    }
 }

-// https://w3c.github.io/FileAPI/#task-read-operation
+// https://w3c.github.io/FileAPI/#thread-read-operation


This needs to be reverted.

jdm · 2015-11-25T19:31:06Z

components/script/dom/filereader.rs


-    let task = box FileReaderEvent::ProcessReadData(filereader.clone(), gen_id);
-    chan.send(CommonScriptMsg::RunnableMsg(FileRead, task)).unwrap();
+    let thread = box FileReaderEvent::ProcessReadData(filereader.clone(), gen_id);


jdm · 2015-11-25T19:31:12Z

components/script/dom/filereader.rs


    let bytes = match blob_contents.recv() {
        Ok(bytes) => bytes,
        Err(_) => {
-            let task = box FileReaderEvent::ProcessReadError(filereader,
+            let thread = box FileReaderEvent::ProcessReadError(filereader,


jdm · 2015-11-25T19:31:18Z

components/script/dom/filereader.rs

            return;
        }
    };

-    let task = box FileReaderEvent::ProcessReadEOF(filereader, gen_id, data, bytes);
-    chan.send(CommonScriptMsg::RunnableMsg(FileRead, task)).unwrap();
+    let thread = box FileReaderEvent::ProcessReadEOF(filereader, gen_id, data, bytes);


jdm · 2015-11-25T19:31:44Z

components/script/dom/websocket.rs

@@ -248,20 +248,20 @@ impl WebSocket {
                Ok(channel) => channel,
                Err(e) => {
                    debug!("Failed to establish a WebSocket connection: {:?}", e);
-                    let task = box CloseTask {
+                    let thread = box CloseThread {


jdm · 2015-11-25T19:31:51Z

components/script/dom/websocket.rs

                    return;
                }
            };
            let ws_sender = Arc::new(Mutex::new(ws_sender));

-            let open_task = box ConnectionEstablishedTask {
+            let open_thread = box ConnectionEstablishedThread {


jdm · 2015-11-25T19:31:57Z

components/script/dom/websocket.rs

@@ -274,19 +274,19 @@ impl WebSocket {
                    Ok(Message::Pong(_)) => continue,
                    Ok(Message::Close(data)) => {
                        ws_sender.lock().unwrap().send_message(Message::Close(data)).unwrap();
-                        let task = box CloseTask {
+                        let thread = box CloseThread {


jdm · 2015-11-25T19:32:02Z

components/script/dom/websocket.rs

                        break;
                    },
                    Err(_) => break,
                };
-                let message_task = box MessageReceivedTask {
+                let message_thread = box MessageReceivedThread {


jdm · 2015-11-25T19:32:10Z

components/script/dom/websocket.rs

@@ -327,11 +327,11 @@ impl WebSocket {
        if !self.clearing_buffer.get() && self.ready_state.get() == WebSocketRequestState::Open {
            self.clearing_buffer.set(true);

-            let task = box BufferedAmountTask {
+            let thread = box BufferedAmountThread {


jdm · 2015-11-25T19:32:27Z

components/script/dom/websocket.rs

    address: Trusted<WebSocket>,
    message: MessageData,
 }

-impl Runnable for MessageReceivedTask {
+impl Runnable for MessageReceivedThread {


jdm · 2015-11-25T19:34:14Z

@bors-servo: delegate+
With those changes and a rebase this should be good to land.

bors-servo · 2015-11-25T19:34:15Z

✌️ @wenderen can now approve this pull request

KiChjang · 2015-12-20T02:54:30Z

@wenderen What's the status of this?

ajnirp · 2015-12-28T18:00:25Z

@KiChjang apologies for the late response, I was on vacation until yesterday.

The rebase is halfway complete (I abandoned the old rebase and re-fetched, so some of my former progress was lost). Should be done by tomorrow night.

ajnirp · 2015-12-29T20:01:09Z

rebased, squashed and pushed

jdm · 2015-12-29T20:23:56Z

@wenderen There are still some incorrect renames that I identified that require addressing, if you've got the time.

ajnirp · 2015-12-29T21:11:51Z

@jdm Sure, which renames?

jdm · 2015-12-29T21:38:49Z

@wenderen See all the comments on the earlier comments further up the page. The ones that are still visible haven't been addressed.

bors-servo · 2015-12-30T03:07:22Z

☔ The latest upstream changes (presumably #9083) made this pull request unmergeable. Please resolve the merge conflicts.

ajnirp · 2015-12-30T05:30:49Z

Oops. How do I reopen this?

frewsxcv · 2015-12-30T09:07:07Z

You might have deleted the branch. It'd probably be easier just to open a new pr

ajnirp · 2016-01-08T16:29:50Z

@frewsxcv done: #9201

highfive added the S-awaiting-review label Nov 13, 2015

jdm self-assigned this Nov 20, 2015

jdm reviewed Nov 25, 2015
View changes

components/script/dom/websocket.rs Outdated

@@ -496,11 +496,11 @@ impl Runnable for ConnectionEstablishedTask {

}

}

struct BufferedAmountTask {

struct BufferedAmountThread {

This comment has been minimized.

Sign in to view

jdm Nov 25, 2015
Member

Please revert this change.

jdm reviewed Nov 25, 2015
View changes

components/script/dom/websocket.rs Outdated

@@ -514,11 +514,11 @@ impl Runnable for BufferedAmountTask {

}

}

struct CloseTask {

struct CloseThread {

This comment has been minimized.

Sign in to view

jdm Nov 25, 2015
Member

Please revert this.

jdm reviewed Nov 25, 2015
View changes

components/script/dom/websocket.rs Outdated

@@ -551,16 +551,16 @@ impl Runnable for CloseTask {

}

}

struct MessageReceivedTask {

struct MessageReceivedThread {

This comment has been minimized.

Sign in to view

jdm Nov 25, 2015
Member

Please revert this.

jdm reviewed Nov 25, 2015
View changes

components/script/dom/websocket.rs Outdated

return;

}

};

let ws_sender = Arc::new(Mutex::new(ws_sender));

let open_task = box ConnectionEstablishedTask {

let open_thread = box ConnectionEstablishedThread {

This comment has been minimized.

Sign in to view

jdm Nov 25, 2015
Member

Revert.

jdm reviewed Nov 25, 2015
View changes

components/script/dom/websocket.rs Outdated

break;

},

Err(_) => break,

};

let message_task = box MessageReceivedTask {

let message_thread = box MessageReceivedThread {

This comment has been minimized.

Sign in to view

jdm Nov 25, 2015
Member

Revert.

jdm reviewed Nov 25, 2015
View changes

components/script/dom/websocket.rs Outdated

address: Trusted<WebSocket>,

message: MessageData,

}

impl Runnable for MessageReceivedTask {

impl Runnable for MessageReceivedThread {

This comment has been minimized.

Sign in to view

jdm Nov 25, 2015
Member

Revert.

jdm added the S-needs-code-changes label Nov 25, 2015

jdm removed the S-awaiting-review label Nov 25, 2015

ajnirp force-pushed the ajnirp:8512-rename-task-to-thread branch from 08f3391 to 7eaf55d Dec 29, 2015

highfive added S-awaiting-review and removed S-needs-code-changes labels Dec 29, 2015

jdm added S-needs-code-changes and removed S-awaiting-review labels Dec 29, 2015

highfive added S-awaiting-review and removed S-needs-code-changes labels Dec 29, 2015

ajnirp force-pushed the ajnirp:8512-rename-task-to-thread branch from 3142e28 to fbd0f85 Dec 29, 2015

highfive added the S-needs-rebase label Dec 30, 2015

ajnirp closed this Dec 30, 2015

ajnirp force-pushed the ajnirp:8512-rename-task-to-thread branch from fbd0f85 to 0f5c614 Dec 30, 2015

ajnirp deleted the ajnirp:8512-rename-task-to-thread branch Jan 8, 2016

jdm mentioned this pull request Feb 5, 2016

Rename *_thread_source to *_task_source #9561

Closed

servo / servo

Join GitHub today

task -> thread #8522

task -> thread #8522

Conversation

ajnirp commented Nov 13, 2015

highfive commented Nov 13, 2015

Manishearth commented Nov 14, 2015

ajnirp commented Nov 14, 2015

jdm commented Nov 14, 2015

ajnirp commented Nov 20, 2015

Manishearth commented Nov 24, 2015

Manishearth commented Nov 24, 2015

ajnirp commented Nov 24, 2015

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jdm commented Nov 25, 2015

bors-servo commented Nov 25, 2015

KiChjang commented Dec 20, 2015

ajnirp commented Dec 28, 2015

ajnirp commented Dec 29, 2015

jdm commented Dec 29, 2015

ajnirp commented Dec 29, 2015

jdm commented Dec 29, 2015

bors-servo commented Dec 30, 2015

ajnirp commented Dec 30, 2015

frewsxcv commented Dec 30, 2015

ajnirp commented Jan 8, 2016