Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(65)

Issue 1880803: Too many open files when generating ClientBundles

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 2 months ago by Colin Alworth
Modified:
3 years, 11 months ago
CC:
Base URL:
http://google-web-toolkit.googlecode.com/svn/trunk
Visibility:
Public.

Description

ImageBundleBuilder and ImageResourceGenerator don't explicitly close files when
finished with them. On some machines, this results in too many files being open
for the process to open additional files, preventing compilation from
continuing. 

Patch Set 1

Total comments: 7

Patch Set 2 : Updated draft reflecting Thomas's comments, including changes to Utility to unify all close methods

Total comments: 5

Patch Set 3 : Updated to emit log message on failed close(), nicer variable names

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
dev/core/src/com/google/gwt/util/tools/Utility.java View 1 2 3 chunks +6 lines, -73 lines 0 comments Download
user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java View 1 2 4 chunks +11 lines, -4 lines 0 comments Download
user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java View 1 2 4 chunks +23 lines, -3 lines 0 comments Download

Messages

Total messages: 19
Colin Alworth
4 years, 2 months ago #1
tbroyer
You might want to try out Git/Gerrit ;-) http://gwt-code-reviews.appspot.com/1880803/diff/1/user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java File user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java (right): http://gwt-code-reviews.appspot.com/1880803/diff/1/user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java#newcode775 user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java:775: if ...
4 years, 2 months ago #2
Colin Alworth
On 2013/01/03 23:46:58, tbroyer wrote: > You might want to try out Git/Gerrit ;-) I ...
4 years, 2 months ago #3
Colin Alworth
http://gwt-code-reviews.appspot.com/1880803/diff/1/user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java File user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java (right): http://gwt-code-reviews.appspot.com/1880803/diff/1/user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java#newcode775 user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java:775: if (input != null) { On 2013/01/03 23:46:58, tbroyer ...
4 years, 2 months ago #4
Colin Alworth
Updated draft reflecting Thomas's comments, including changes to Utility to unify all close methods
4 years, 2 months ago #5
jens
http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java File dev/core/src/com/google/gwt/util/tools/Utility.java (right): http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java#newcode54 dev/core/src/com/google/gwt/util/tools/Utility.java:54: } catch (IOException e) { How about logging this ...
4 years, 2 months ago #6
tbroyer
LGTM (BTW why was the GWT-Contrib group removed from CC?) http://gwt-code-reviews.appspot.com/1880803/diff/1/user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java File user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java (right): http://gwt-code-reviews.appspot.com/1880803/diff/1/user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java#newcode778 ...
4 years, 2 months ago #7
jens
http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java File dev/core/src/com/google/gwt/util/tools/Utility.java (right): http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java#newcode54 dev/core/src/com/google/gwt/util/tools/Utility.java:54: } catch (IOException e) { On 2013/01/04 02:26:08, tbroyer ...
4 years, 2 months ago #8
tbroyer
http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java File dev/core/src/com/google/gwt/util/tools/Utility.java (right): http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java#newcode54 dev/core/src/com/google/gwt/util/tools/Utility.java:54: } catch (IOException e) { On 2013/01/04 10:00:17, jens ...
4 years, 2 months ago #9
jens
http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java File dev/core/src/com/google/gwt/util/tools/Utility.java (right): http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java#newcode54 dev/core/src/com/google/gwt/util/tools/Utility.java:54: } catch (IOException e) { On 2013/01/04 10:05:28, tbroyer ...
4 years, 2 months ago #10
Colin Alworth
Updated to emit log message on failed close(), nicer variable names
4 years, 2 months ago #11
Colin Alworth
On 2013/01/04 13:03:22, jens wrote: > http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java > File dev/core/src/com/google/gwt/util/tools/Utility.java (right): > > http://gwt-code-reviews.appspot.com/1880803/diff/7001/dev/core/src/com/google/gwt/util/tools/Utility.java#newcode54 > ...
4 years, 2 months ago #12
jens
On 2013/01/10 19:30:33, Colin Alworth wrote: > On 2013/01/04 13:03:22, jens wrote: > > > ...
4 years, 2 months ago #13
icfantv
Is there any update on this patch and the related bug (7880)? Any chance it ...
4 years ago #14
gottfried.ganssauge
On 2013/01/10 19:21:01, Colin Alworth wrote: > Updated to emit log message on failed close(), ...
3 years, 11 months ago #15
tbroyer
On 2013/04/23 16:15:35, gottfried.ganssauge wrote: > On 2013/01/10 19:21:01, Colin Alworth wrote: > > Updated ...
3 years, 11 months ago #16
tbroyer
On 2013/04/23 16:15:35, gottfried.ganssauge wrote: > On 2013/01/10 19:21:01, Colin Alworth wrote: > > Updated ...
3 years, 11 months ago #17
gottfried.ganssauge
On 2013/04/23 16:28:16, tbroyer wrote: > On 2013/04/23 16:15:35, gottfried.ganssauge wrote: > > On 2013/01/10 ...
3 years, 11 months ago #18
tbroyer
3 years, 11 months ago #19
On 2013/04/24 07:40:12, gottfried.ganssauge wrote:
> 
> Actually I was under the impression that it's probably best to add to *this*
> review instance.
> If I'm right the OP would need to post my enhanced patch set as I haven't seen
> how to do this myself.

Yes, with Rietveld only the "issue" creator can add new patchsets.

> Am I totally misleaded when I think that creating a git+gerrit review would
mean
> to create a new issue? And wouldn't that issue conflict with this one?

No worries, there won't be any conflict; whether you create a new "change" in
Gerrit or a new "issue" in Rietveld.
Just post the link of your review here to make sure Colin's patch won't be
merged and yours will be reviewed instead. And in case Colin's patch is merged,
you can then update your review (with git, simply rebase on master, solving the
merge conflicts).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld revision f51cb906c4ad+