ClassReader and BufWriter

[Maurizio Cimadamore]

On 09/02/2023 14:15, Brian Goetz wrote:
>
>
> On 2/9/2023 7:49 AM, Maurizio Cimadamore wrote:
>> Hi,
>> as I'm looking at the classfile PR (great work!), I have few 
>> questions on ClassReader and BufWriter:
>>
>> * I believe they are only used for attribute reading/writing - so 
>> perhaps they should belong to the `attribute` package?
>
> They are used for more than attribute reading and writing. BufWriter 
> is used, for example, by DirectClassBuilder::build to build the entire 
> classfile -- the ClassFile, constant pool, method_info structures, 
> field_info structures, etc.  (Because we don't always know the sizes, 
> to minimize buffer copies, we write pieces of the classfile in chunks 
> and join them.  DBC::build has two BWs for this, one for the "head" 
> (magic number, fixed ClassFile fields, constant pool) and one for the 
> tail (fields, methods, attributes), because we don't know how big the 
> CP will be until we finish rendering the entire "tail".
Sure, but DirectClassBuilder is an implementation class. I'm talking 
about API usages here. It seems to me that BufWriter is an 
implementation abstraction that "happens" to implement the interface for 
attribute writing (which is the only place where the user needs to write 
things by hand).
>
> A BufWriter would be a pure buffer abstraction -- it is 99% about just 
> writing various kinds of data to a growable buffer -- but it does 
> carry with it a reference to a constant pool builder (which may be 
> shared across multiple BufWriters if they are writing different chunks 
> of the same classfile), since it needs to be able to write not only 
> ints, but constant pool indexes, and this might require growing the 
> constant pool to add a new entry.
>
>> * BufWriter should be renamed ClassWriter (as it's the dual?)
>
> Yes, but no :)  This code got refactored a number of times (including 
> one like what you suggest), but the naming asymmetry derives from a 
> functional asymmetry.  We write classfiles in lots of small chunks -- 
> we accumulate the bytes of constant pools, methods, attributes, etc in 
> their own little buffers -- but we read the classfile monolithically.  
> (We have to, because you can't even find the methods until you've 
> parsed the constant pool.)  So ClassReader is tied to a single byte[] 
> which represents the whole classfile, and manages the constant pool, 
> navigation, byte access, everything -- whereas BufWriter only knows 
> about writing to a buffer, plus uses the service of a 
> ConstantPoolBuilder to render constant pool offsets.
>
> So the two do exist as different levels of abstraction.
I get the subtle, implementation-driven distinction - but I find 
BufWriter a rather weak name API-wise. That said, this stuff is subjective.
>
>> While I understand this is in bikeshed territory, I think it would be 
>> nice if the number of classes in the main package was kept as low as 
>> possible, to let the true nature of the API (models, elements, 
>> builders) stand out more.
>
> I did consider this organizational question.  The `attributes` package 
> is easy, because it contains almost exclusively model classes for the 
> various attributes, which are derived almost mechanically from JVMS.  
> You could arguably move a few things like BufWriter (which mostly 
> serve the implementation) into a `util` package, but this wouldn't 
> really do very much to declutter the main package; there are just a 
> lot of different kinds of entities (methods, fields, classes, 
> attributes, constant pool entries, instructions, plus elements of the 
> classfile header like Superclass) in a classfile, and each gets 
> models, builders, elements, transforms, etc.  These feel like they go 
> together.  So it felt that this process reached diminishing returns 
> the last time I tried this.
Maurizio
>
>