Skip to content
This repository has been archived by the owner on Dec 10, 2022. It is now read-only.

Update timer #271

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Update timer #271

wants to merge 4 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 13, 2017

No description provided.

@ghost ghost added this to the 0.6.8 milestone Sep 13, 2017
@ghost ghost self-assigned this Sep 13, 2017
@ghost ghost requested a review from soywiz September 13, 2017 08:26
void add(TimerTask task) {
// Grow backing store if necessary
if (size + 1 == queue.length)
queue = Arrays.copyOf(queue, 2*queue.length);
Copy link
Member

Choose a reason for hiding this comment

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

Since those are not integers or other primitives, why not to use an ArrayList instead? That way you would avoid having to do this manually

Copy link
Member

@soywiz soywiz left a comment

Choose a reason for hiding this comment

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

If you need this already, feel free to merge, I don't see anything too strange. But we should create an issue to revisit this. Maybe converting some manual arrays into ArrayList/Heap, and in the future maybe providing a way to use native timers in targets like JS?

Copy link
Contributor

@intrigus intrigus left a comment

Choose a reason for hiding this comment

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

What's the license of those files?

}

if (!taskFired) { // Task hasn't yet fired; wait
Thread.sleep(50);
Copy link
Member

Choose a reason for hiding this comment

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

This one is another target for a future optimization using synchronization primitives. I have to check the status of them right now, since the initial target of jtransc was single threaded environments probably they still need some work, so I can understand this if they are not fully working by now.

@codecov
Copy link

codecov bot commented Sep 13, 2017

Codecov Report

Merging #271 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #271   +/-   ##
=========================================
  Coverage     17.89%   17.89%           
  Complexity      388      388           
=========================================
  Files           312      312           
  Lines         20749    20749           
  Branches       4495     4495           
=========================================
  Hits           3713     3713           
  Misses        16360    16360           
  Partials        676      676

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfee8bd...c9bddb8. Read the comment docs.

@ghost
Copy link
Author

ghost commented Sep 13, 2017

@intrigus you right. i found text of header and insert it.

@intrigus
Copy link
Contributor

@soywiz I am not sure but I think that we can not mix Apache 2.0 and GPL Classpath Exception.

@soywiz
Copy link
Member

soywiz commented Sep 13, 2017

Im not a license expert. I think that java.util base comes from android.
Thats why i tried to provide stubs initially and then implement non criticsl packages myself.
Sergey, why do you need this change?

What i can do is to try to use either android or openjdk as base. But it will require modifications to not include lots of stuff, not being that dependant on threads for jslike targets, and to reduce interdpendency and not use if that comptible with openjdk. Also until java8 support is not completed (mostly the pending PR) it wouldnt work.

@ghost
Copy link
Author

ghost commented Sep 13, 2017

I try fix deadlocks in my app after sync added for lime =( we can - not merge this, if i can fix i write about here.

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