[
https://issues.apache.org/jira/browse/AVRO-1642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14738075#comment-14738075
]
Sean Busbey commented on AVRO-1642:
-----------------------------------
{code}
+ /*
+ * Returns the number of parameter units required by fields for the
AllArgsConstructor
+ */
+ protected int calcAllArgConstructorParameterUnits(Schema record) {
{code}
Add a javadoc that the schema needs to be a Record, add an input validation
that the schema is a Record, add a failure test for same.
{code}
+ switch(f.schema().getType()){
+ case DOUBLE:
+ case FLOAT:
+ parameterUnits += 2; // double & float types contribute 2 parameter
units
+ break;
+ default:
+ parameterUnits += 1; // all other types contribute 1 parameter unit
+ }
{code}
Types that need a java long or double count as 2, everything else is 1.
Important to note that it's only the primitive types long and double.
Per [generic java api
docs|http://avro.apache.org/docs/1.7.7/api/java/org/apache/avro/generic/package-summary.html],
Avro is going to generate Double and Long objects instead, which should count
as 1 parameter unit.
My apologies for pointing you towards the JVM note earlier. For some reason at
the time I thought specific would use primitives.
{code}
+ /*
+ * @throws RuntimeException generated code for specified record will be not
compile.
+ */
+ protected void validateRecordForCompilation(Schema record) {
+ this.createAllArgsConstructor =
+ calcAllArgConstructorParameterUnits(record) <=
MAX_FIELD_PARAMETER_UNIT_COUNT;
+ }
{code}
please remove the javadoc throws, since this does not throw. Also please log a
warning when createAllArgsConstructor will be false.
{code}
+#if ($this.isCreateAllArgsConstructor())
/**
* All-args constructor.
@@ -80,6 +81,7 @@ public class ${this.mangle($schema.getName())}#if
($schema.isError()) extends or
#end
}
#end
+#end
{code}
When we're not going to create the all-args constructor, please output a java
comment that explains why we aren't making it, a pointer to using the builder
pattern, etc.
{quote}
rename of
lang/java/compiler/src/test/java/org/apache/avro/compiler/TestSpecificCompiler.java
to
lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
{quote}
please don't include the rename in this patch, since it makes it hard to see
what changes are relevant to this patch (we should do this rename in a
different JIRA, we can do it first if it makes this patch easier).
{code}
+ @Test
+ public void testMaxParameterCounts() throws Exception {
+ Schema validSchema1 =
createSampleRecordSchema(SpecificCompiler.MAX_FIELD_PARAMETER_UNIT_COUNT, 0);
+ new SpecificCompiler(validSchema1).compile();
+ Schema validSchema2 =
createSampleRecordSchema(SpecificCompiler.MAX_FIELD_PARAMETER_UNIT_COUNT - 2,
1);
+ new SpecificCompiler(validSchema2).compile();
+ Schema validSchema3 =
createSampleRecordSchema(SpecificCompiler.MAX_FIELD_PARAMETER_UNIT_COUNT - 1,
1);
+ new SpecificCompiler(validSchema3).compile();
+ Schema validSchema4 =
createSampleRecordSchema(SpecificCompiler.MAX_FIELD_PARAMETER_UNIT_COUNT + 1,
0);
+ new SpecificCompiler(validSchema4).compile();
+ }
{code}
Include comments of what you are attempting to test in these calls,
expectations of success/failure, etc.
{code}
-
+
/** Uses the system's java compiler to actually compile the generated code.
*/
- static void assertCompilesWithJavaCompiler(Collection<OutputFile> outputs)
+ static void assertCompilesWithJavaCompiler(Collection<OutputFile> outputs)
throws IOException {
if (outputs.isEmpty()) {
return; // Nothing to compile!
@@ -730,13 +757,13 @@ public class TestSpecificCompiler {
}
JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
- StandardJavaFileManager fileManager =
- compiler.getStandardFileManager(null, null, null);
-
- CompilationTask cTask = compiler.getTask(null, fileManager, null, null,
- null,
- fileManager.getJavaFileObjects(
+ StandardJavaFileManager fileManager =
+ compiler.getStandardFileManager(null, null, null);
+
+ CompilationTask cTask = compiler.getTask(null, fileManager, null, null,
+ null, fileManager.getJavaFileObjects(
javaFiles.toArray(new File[javaFiles.size()])));
- assertTrue(cTask.call());
+ boolean compilesWithoutError = cTask.call();
+ assertTrue(compilesWithoutError);
{code}
Please leave out unrelated code reformatting.
{quote}
lang/java/tools/src/test/compiler/output-no-constructor/*
{quote}
Where are these coming from / used?
> JVM Spec Violation 255 Parameter Limit Exceeded
> ------------------------------------------------
>
> Key: AVRO-1642
> URL: https://issues.apache.org/jira/browse/AVRO-1642
> Project: Avro
> Issue Type: Bug
> Components: java
> Affects Versions: 1.7.7
> Environment: Windows/Linux all Java
> Reporter: Bryce Alcock
> Assignee: Prateek Rungta
> Priority: Critical
> Labels: build, maven, specific
> Attachments: AVRO-1642-0.patch, AVRO-1642-1.patch, avro-1642-fail.tar
>
>
> The JVM Spec indicates that:
> {quote}The number of method parameters is limited to 255 by the definition of
> a method descriptor (ยง4.3.3), where the limit includes one unit for this in
> the case of instance or interface method invocations. Note that a method
> descriptor is defined in terms of a notion of method parameter length in
> which a parameter of type long or double contributes two units to the length,
> so parameters of these types further reduce the limit. {quote}
> Avro Generated Java code with say more than 255 fields will create a
> constructor that is not valid and won't compile.
> Simple test is to create a 256 field avro schema, use the avro-maven auto
> code gen plugin, and try to compile the resulting class.
> DON'T use linux when doing this use windows, my suspicion is that Linux JavaC
> generates invalid byte code but does not complain.
> Windows will correctly complain indicating that you are a violator of the JVM
> specification.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
