Skip to content

索引超限问题#9

Merged
hcsp-bot merged 8 commits intomasterfrom
YY
Sep 15, 2019
Merged

索引超限问题#9
hcsp-bot merged 8 commits intomasterfrom
YY

Conversation

@Scott-YuYan
Copy link
Contributor

有个索引超限问题不知道怎么解决

@hcsp-bot
Copy link
Contributor

🎉 感谢提交Pull Request!请稍等片刻,我们已经将其提交到CI进行检查,一旦有结果会立即通知您!
不过,我们发现你在这个仓库中还打开了其他的Pull Request:

#8

我们不鼓励同时打开多个Pull Request,请集中精力于现在的这个Pull Request,谢谢!:pray:

@hcsp-bot
Copy link
Contributor

你的提交 ed04930 ,似乎失败了:Your tests failed on CircleCI

😅 请不要气馁,仔细分析原因,再接再厉!

@@ -1,14 +1,77 @@
package com.github.hcsp.multithread;

import com.sun.deploy.security.SelectableSecurityManager;
Copy link
Collaborator

Choose a reason for hiding this comment

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

这是什么鬼

List<Future<Map<String, Integer>>> list = new ArrayList<>();
Map<String, Integer> finalResult = new HashMap<>();
for (int i = 0; i < threadNum; ++i) {
while (files.get(0).exists()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里如果files为空呢?

Suggested change
while (files.get(0).exists()) {
while (!files.isEmpty()) {

Copy link
Collaborator

Choose a reason for hiding this comment

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

我这样建议只是指出你的问题,并不代表这样改了你的程序就能正常工作了。你最好避免多个子线程同时操作List,原因是:

  1. 你的List没有同步。
  2. 你怎么保证List是可以修改的,就调用它的remove方法?
  3. 很容易出线程安全问题。

}
}
files.remove(0);
threadPool.shutdown();
Copy link
Collaborator

Choose a reason for hiding this comment

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

每个子线程执行完就自己把自己的线程池关掉?这个操作很骚啊。

Copy link
Collaborator

Choose a reason for hiding this comment

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

虽然你从线程池中的线程里关闭肯定会遇到异常,但是我还是想问,线程1执行完了就把线程池整个关掉,那别的线程要是还在执行咋办?

Copy link
Collaborator

@blindpirate blindpirate left a comment

Choose a reason for hiding this comment

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

最后的一个建议是,别写这么长的方法,因为很难理解,人类的思维可能跟不上。我的建议是,你声明一个这样的内部类:

static class Worker implements Callable<Map<String, Integer>> {
    List<File> 子线程分配到的一两个文件;
    @Override
    ...
}


// 在count方法中
// 先把文件列表按照线程数量切割成若干个子列表
threadPool.submit(new Worker(这个子线程分配到的文件子列表))

@Scott-YuYan
Copy link
Contributor Author

Scott-YuYan commented Sep 14, 2019 via email

@Scott-YuYan
Copy link
Contributor Author

Scott-YuYan commented Sep 14, 2019 via email

@hcsp-bot
Copy link
Contributor

你的提交 ee933fd ,似乎失败了:Your tests failed on CircleCI

😅 请不要气馁,仔细分析原因,再接再厉!


for (Future<Map<String, Integer>> future : list
) {
Map<String, Integer> resultFromThread = future.get();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@blindpirate 老师,我调试到这里的时候,发现就只提示了一个 application is running了。想问下是为什么呢?

Copy link
Collaborator

Choose a reason for hiding this comment

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

你难道没注意到你的电脑CPU负载很高么?所有的线程都在跑死循环啊。。。

}
}

private synchronized static List<List<File>> averageList(List<File> files, Integer number) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@blindpirate 这个文件拆分的方法还有些问题

Copy link
Collaborator

Choose a reason for hiding this comment

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

我有没有跟你说过,不要自己造轮子?自己尝试搜索一下有没有类似方法?如果你需要的话,我可以允许你修改pom.xml.

public Map<String, Integer> call() throws Exception {
Map<String, Integer> result = new ConcurrentHashMap<>();
while (!files.isEmpty()) {
for (File file : files) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个方法的时间复杂度是太高了,最开始的时候我是用files.remove(index)读取list中的文件的,会报出ConcurrentModificationException,尝试了加锁,已经copyOnWriteArrayList几种方法都没效。

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个时间复杂度有啥高的?

@Override
public Map<String, Integer> call() throws Exception {
Map<String, Integer> result = new ConcurrentHashMap<>();
while (!files.isEmpty()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

把这个while去掉就好了。不明白为啥你要套两层循环:

  1. while
  2. for (File file : files)

因为files永远不为空,所以这个循环永远不会结束。

@Scott-YuYan
Copy link
Contributor Author

Scott-YuYan commented Sep 15, 2019 via email

@hcsp-bot
Copy link
Contributor

你的提交 a219aab ,似乎失败了:Your tests failed on CircleCI

😅 请不要气馁,仔细分析原因,再接再厉!

@hcsp-bot
Copy link
Contributor

你的提交 7dd9a6c ,似乎失败了:Your tests failed on CircleCI

😅 请不要气馁,仔细分析原因,再接再厉!

@hcsp-bot
Copy link
Contributor

你的提交 8500bed ,似乎失败了:Your tests failed on CircleCI

😅 请不要气馁,仔细分析原因,再接再厉!

@hcsp-bot
Copy link
Contributor

恭喜你,你的提交 9ebc882 已经通过我们的CI检查:Your tests passed on CircleCI!

👍 它会被自动merge后revert。请不要骄傲,继续挑战!

@hcsp-bot hcsp-bot merged commit 076dc0e into master Sep 15, 2019
hcsp-bot added a commit that referenced this pull request Sep 15, 2019
public class WordCount {
public WordCount(int threadNum) {}
private final int threadNum;
private ExecutorService threadPool;
Copy link
Collaborator

Choose a reason for hiding this comment

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

所以这玩意为什么不是final的呢。。。


@Override
public Map<String, Integer> call() throws Exception {
Map<String, Integer> result = new ConcurrentHashMap<>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

其实这里就没必要用ConcurrentHashMap了,想一想为什么?

Copy link
Collaborator

Choose a reason for hiding this comment

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

因为这是一个局部变量啊,没有任何并发写操作发生啊。

@Scott-YuYan
Copy link
Contributor Author

Scott-YuYan commented Sep 15, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants