Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Prevent creation of unlimited number of compile workers #198

Closed
0mkara opened this issue Oct 7, 2018 · 18 comments
Closed

Prevent creation of unlimited number of compile workers #198

0mkara opened this issue Oct 7, 2018 · 18 comments
Assignees
Labels

Comments

@0mkara
Copy link
Owner

0mkara commented Oct 7, 2018

Currently when solidity files are saved or changed frequently, it results creation of unlimited number of compile workers. This crashes the computer.

@0mkara 0mkara added the bug label Oct 7, 2018
@0mkara 0mkara self-assigned this Oct 7, 2018
@0mkara
Copy link
Owner Author

0mkara commented Nov 2, 2018

  • There should be always a finite number of node child process that can be initiated from Etheratom to run specific tasks.
  • User should be able to cancel previous running task if any and start new task.
  • A task runner indicating running tasks & able to cancel current running process should be implemented as well.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 150.0 DAI (150.0 USD @ $1.0/DAI) attached to it as part of the ECF fund.

@gitcoinbot
Copy link

gitcoinbot commented Nov 15, 2018

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work by 2 months, 3 weeks ago.
Please review their action plans below:

1) deleafgreen has applied to start work (Funders only: approve worker | reject worker).

Get access to project and reduce number oft workers

Learn more on the Gitcoin Issue Details page.

2) jeremigendron has been approved to start work.

I've submitted a basic PR outlining what I think is an OK approach to fixing the problem. It is by no means a complete solution but might offer grounds for another developer to make some progress on the issue. It is untested. #217.

Learn more on the Gitcoin Issue Details page.

@tcrowe
Copy link

tcrowe commented Nov 16, 2018

Hi @JeremiGendron. I looked at your PR and it could work. I wonder if it would be better to find out what is causing the fork before even starting it. For example adding debugger; into compileWeb3 to see the stack trace. Using a debounce might help.

Some places to look:

async compileWeb3(sources) {

this.helpers.compileWeb3(sources);

this.compileSubscriptions.add(atom.workspace.observeTextEditors((editor) => {


I'm just a guy looking around at issues. You don't have to do anything based on what I say.

@tpscrpt
Copy link
Contributor

tpscrpt commented Nov 16, 2018 via email

@0mkara
Copy link
Owner Author

0mkara commented Nov 17, 2018

what is causing the fork before even starting it

@tcrowe can you explain what do you mean ?

@JeremiGendron your approach of limiting total number of workers to 10 is quite simple and good. I also tried similar issue and it kinda does work. However there are few other issues I saw when I tried this solution earlier.

  • Imagine you have submitted a.sol for compilation [i.e hit save and etheratom has started to compile it]. Now while rapidly developing you move to file b.sol and hit save 2 times. Now 2 more jobs has started. Then the first job finishes and the result you see, is compilation result for a.sol. This is quite confusing for users. Users would be expecting results for b.sol.
  • When a compilation is going on there is no interactive way to stop that job. This is a required feature.
  • There is a problem with the save event of atom. Even if I hit save shortcut button ctrl+s once I could see multiple event being triggered and that caused creation of multiple compiler worker when we should expect only one worker.

I think solution for this issue would be:

  • find out the root cause of firing multiple events from save event in the editor and prevent creation of multiple workers
  • limit maximum numbers of workers
  • provide user interface to kill or cancel jobs

@JeremiGendron @tcrowe thank you both for looking into Etheratom IDE issues and contribute to them.

@tpscrpt
Copy link
Contributor

tpscrpt commented Nov 17, 2018 via email

@tpscrpt
Copy link
Contributor

tpscrpt commented Nov 17, 2018

#217 has been update with a few changes, will now devote some time to setting up the environment and tuning it.

@tcrowe
Copy link

tcrowe commented Nov 18, 2018

@0mkara I think you guys understood well. This problem comes up ALL the time when forking.

As an Atom user I am looking forward to trying this extension! Thanks guys :D

@tpscrpt
Copy link
Contributor

tpscrpt commented Nov 19, 2018

@0mkara I'll be finishing up the theory and PoC tonight

@spm32
Copy link

spm32 commented Nov 21, 2018

Great work so far @JeremiGendron, just approved you formally to continue for the bounty!

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work for 150.0 DAI (150.0 USD @ $1.0/DAI) has been submitted by:

  1. @jeremigendron

@ceresstation please take a look at the submitted work:

  • PR by @JeremiGendron

@0mkara
Copy link
Owner Author

0mkara commented Nov 24, 2018

PR #217 has been merged and published at a6a8225

@0mkara 0mkara closed this as completed Nov 24, 2018
@tpscrpt
Copy link
Contributor

tpscrpt commented Nov 24, 2018

@ceresstation Have you reviewed the submitted work on gitcoin?

@0mkara
Copy link
Owner Author

0mkara commented Nov 24, 2018

No. I don't have any mod permission on gitcoin. I will contact @ceresstation.

@spm32
Copy link

spm32 commented Nov 25, 2018

Sorry for the delay @JeremiGendron, awesome job, paying you out now!

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


The funding of 150.0 DAI (150.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @JeremiGendron.

@tpscrpt
Copy link
Contributor

tpscrpt commented Nov 25, 2018

@ceresstation much love, thanks!

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

No branches or pull requests

5 participants