Skip to content
This repository has been archived by the owner on Sep 23, 2021. It is now read-only.

Refactoring connections #74

Merged
merged 9 commits into from
Jun 8, 2016
Merged

Refactoring connections #74

merged 9 commits into from
Jun 8, 2016

Conversation

dancrumb
Copy link
Contributor

@dancrumb dancrumb commented Jun 7, 2016

So, this is a whole host of moving code around.

As I was steadily understanding how things worked, I started renaming variable and functions, or extracting stuff, so that the code more readily explained itself.

I noticed a lot of code where Object A was making modifications to Object C or D's properties, even though they were owned by Object B.

I've tried to rejig things so that there is a little more encapsulation.

I also beefed up the tracing.

Finally, after all this, I did some profiling and made the odd tweak here and there.

All in all, it's mostly just relocation of code. I understand that it's a chunk of work, but I hope it adds clarity to you as well as me.

@dancrumb
Copy link
Contributor Author

dancrumb commented Jun 7, 2016

Ignore this PR for now... I've got a little more work to do, then I want to add some notes to the PR itself.

@jpaulm
Copy link
Owner

jpaulm commented Jun 7, 2016

No problem! This is fantastic!

Bert Madeira is considering some repackaging of JavaFBP - is this something
you would be interested in participating in or advising on?

Regards,

Paul

On Tue, Jun 7, 2016 at 12:37 AM, Dan Rumney notifications@github.com
wrote:

Ignore this PR for now... I've got a little more work to do, then I want
to add some notes to the PR itself.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#74 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AATGJ5kTFeE2IYKJPZ2SDqmsWdKzzhocks5qJPWHgaJpZM4IviIh
.

@dancrumb
Copy link
Contributor Author

dancrumb commented Jun 7, 2016

Appreciate the offer, but Java is not a strength of mine and there are a few more things I'd like to make sure are supported by JSFBP

@@ -4,3 +4,4 @@ node_modules/
npm-debug.log
test/components/goodbye-world.txt
examples/data/text_new.txt
examples/isolate-*-v8.log
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the logs that the node profiler generates, so we can ignore them

@dancrumb
Copy link
Contributor Author

dancrumb commented Jun 7, 2016

OK - this is ready for your reviewing

This was referenced Jun 7, 2016
try {
runtime.run(_.values(this._processes), options, callback );
} catch (e) {
console.log('Connections');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am finding this very useful. When there's a deadlock, show all the connections and which ones are full. This may help identify why there's a deadlock.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good - is this integrated with my deadlock display - or does it
replace it?!

On Wed, Jun 8, 2016 at 11:04 AM, Dan Rumney notifications@github.com
wrote:

In core/Network.js
#74 (comment):

@@ -86,12 +87,20 @@ Network.prototype.getProcessByName = function (processName) {

Network.prototype.run = function (runtime, options, callback) {

options = options || {};

  • _.forEach(this._processes, function (process) {
  • _.invokeMap(process.inports, 'setRuntime', runtime);
  • _.invokeMap(process.outports, 'setRuntime', runtime);
  • });
  • runtime.run(_.values(this._processes), options, callback || function () {});
  • callback = callback || function () {};
  • _.invokeMap(this._connections, 'setRuntime', runtime);
  • try {
  • runtime.run(_.values(this._processes), options, callback );
  • } catch (e) {
  • console.log('Connections');

I am finding this very useful. When there's a deadlock, show all the
connections and which ones are full. This may help identify why there's
a deadlock.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/jpaulm/jsfbp/pull/74/files/d12c2b94b326314da1d5fe633ff423f329d25d0b#r66273523,
or mute the thread
https://github.com/notifications/unsubscribe/AATGJzMTFg7DyGNxVKEd04O0zZJkEXXbks5qJtoOgaJpZM4IviIh
.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then runtime still displays the deadlock details. Network then catches the exception that gets thrown and shows the connection details.

@jpaulm jpaulm merged commit 9536f94 into master Jun 8, 2016
@jpaulm jpaulm deleted the refactoring-connections branch June 8, 2016 17:15
@jpaulm
Copy link
Owner

jpaulm commented Jun 8, 2016

Came across message "No process passed to input port" , apparently generated by InputPort.js . Since InputPort is not specified by the developer, it seems unlikely that it will be called with a missing or null parameter...? Just curious...

@dancrumb
Copy link
Contributor Author

dancrumb commented Jun 8, 2016

What were you running when you saw that message?

@jpaulm
Copy link
Owner

jpaulm commented Jun 9, 2016

npm test

Here is the output from InputPort.js:

InputPort
No process passed to input port: undefined
Accessing IP types from IP object directly is deprecated. Please use IP.Types.NO
RMAL
V can receive IIPs
No process passed to input port: undefined
V returns null from a closed connection

@dancrumb
Copy link
Contributor Author

dancrumb commented Jun 9, 2016

Strange. You're on the master branch, I assume? And you're up to date (git
pull)?

On Wed, Jun 8, 2016, 20:54 Paul Morrison notifications@github.com wrote:

npm test

Here is the output from InputPort.js:

InputPort
No process passed to input port: undefined
Accessing IP types from IP object directly is deprecated. Please use
IP.Types.NO
RMAL
V can receive IIPs
No process passed to input port: undefined
V returns null from a closed connection


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#74 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AANhTzZBFwwUiLbe1h_MSfMrwNRZfE6mks5qJ3JygaJpZM4IviIh
.

@dancrumb
Copy link
Contributor Author

dancrumb commented Jun 9, 2016

Disregard that last comment... I see them.

I'll take a look at the tests

On Wed, Jun 8, 2016, 23:20 Dan Rumney dancrumb@gmail.com wrote:

Strange. You're on the master branch, I assume? And you're up to date (git
pull)?

On Wed, Jun 8, 2016, 20:54 Paul Morrison notifications@github.com wrote:

npm test

Here is the output from InputPort.js:

InputPort
No process passed to input port: undefined
Accessing IP types from IP object directly is deprecated. Please use
IP.Types.NO
RMAL
V can receive IIPs
No process passed to input port: undefined
V returns null from a closed connection


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#74 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AANhTzZBFwwUiLbe1h_MSfMrwNRZfE6mks5qJ3JygaJpZM4IviIh
.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants