eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.27k stars 721 forks source link

Clean up comments in JIT compiler code that might be treated as documentation comments by Doxygen #17315

Open hzongaro opened 1 year ago

hzongaro commented 1 year ago

Doxygen treats a number of different forms of comment blocks as comments that are intended to be included in documentation that it generates. Among these are comment lines that begin with three slash characters (also sometimes referred to as virgule, solidus and oblique, among other names).

There are many places in the OpenJ9 JIT compiler's source code where code that has been commented out or remarks to which the author intended to draw attention begin with ///. These were not generally intended to be included in HTML generated by Doxygen.

For instance, the following line appears in the source code of J9::CodeGenerator::compressedReferenceRematerialization:

                  ///traceMsg(comp(), "5 ttNode %p\n", ttNode);

Running Doxygen on the source tree, the following results:

◆ compressedReferenceRematerialization()

void J9::CodeGenerator::compressedReferenceRematerialization | ( |   | ) |   -- | -- | -- | -- | --

traceMsg(comp(), "5 ttNode %p\n", ttNode);

traceMsg(comp(), "5 ttNode %p\n", ttNode);

Aman254 commented 1 year ago

Hey! @hzongaro I am Super Enthusiastic to solve this please Assign it to me.

Aman254 commented 1 year ago

@hzongaro As You provided the link so i have to document only the codebase which is linked or the all the codes base that is in the folder of codegen?

hzongaro commented 1 year ago

Hi, @Aman254. That was just one example of the problem. I would recommend focusing on everything under the OpenJ9 runtime/compiler folder and all of its subfolders for the purposes of this issue.

Aman254 commented 1 year ago

@hzongaro sure I'll Look into it

Aman254 commented 1 year ago

Hey @hzongaro I Cleaned up all comments of runtime folder and documented it in a HTML file.

Now I need to create a PR but As am a Beginner Am confused to which branch to commit choose as there are branches called :

  1. jitaas
  2. snapshot
  3. release

so, in this which one should I choose to commit {image-link} https://user-images.githubusercontent.com/87572468/236125649-70a5bf19-6519-46e1-9be3-bf168bf43675.png

Aman254 commented 1 year ago

Hey @hzongaro I sucessfully documented all the comments in runtime folder in form of HTML Document using Doxygen But! I need some help as am Beginner I Don't know where to commit my changes can you help for that please I have the HTML file Ready Documented.

Aman254 commented 1 year ago

Here's the Documented file Ready image

hzongaro commented 1 year ago

Hi, @Aman254. Thanks for the speedy work!

As @RSalman mentioned in your Slack thread, you will need to create a fork of the https://github.com/eclipse-openj9/openj9.git repository and open a pull request.

Aman254 commented 1 year ago

hey @hzongaro Very soon I'll be creating a P.R Its Just taking time because am beginner and learning how create a branch and create a PR. And soo sorry for the delay

0xdaryl commented 1 year ago

Hello @Aman254... Thank you for the contribution. There is a document in the upstream Eclipse OMR project that might help you with GitHub and creating a pull request. It is largely applicable to OpenJ9 as well, I think. Please see https://github.com/eclipse/omr/blob/master/doc/GitCrashCourse.md

Aman254 commented 1 year ago

@0xdaryl sure I'll look forward to it

Aman254 commented 1 year ago

Hi @0xdaryl @hzongaro

help

image image image

Here in the Issue #17315 as it is mentioned that to Document the Comments of the Compiler folder in form of HTML Which is Done. Now Am Unable to understand what Clean up Comments in JIT Compiler refers to ? Does it Mean that I should remove all the comments that are there in the compiler ? And Also Documentation id DONE ? Please Help as am a New to this .

hzongaro commented 1 year ago

Hi, @Aman254. Let me begin by apologizing if the original description of the problem wasn't very clear, or if this comment ends up providing background information of which you are already aware.

There has been some effort going on for several years to add Doxygen-style[1] comments to the APIs in the JIT compiler. This work is nowhere near complete. The most commonly used form for comments that are treated specially by Doxygen in generating HTML uses a slash followed by two asterisks at the start of a block comment, and that's the form the OpenJ9 JIT compiler uses for the most part. For example,

/**
 * \brief This is my method
 * \param[in] arg1 An argument of type int
 * \return A result of type in
 */
 int myMethod(int arg1)

In adding Doxygen-style comments to a method recently and running Doxygen, I noticed that comments in the code that began with three slashes - /// - were included in the HTML generated by Doxygen. The documentation on Doxygen comment blocks[2] indicates that comment blocks whose lines begin with three slashes are treated as Doxygen comment blocks. I found several places in the compiler source code where such comments appear.

The purpose of this issue was to point out that problem. In order to fix the problem, you will need to find the places in the runtime/compiler folder, and its subfolders, where lines begin with ///, and decide whether they were meant to be treated as Doxygen comments.

If any were intended to be treated as Doxygen comments, it would probably be better to change them to use the /** form of comment block. Any that were not intended to be treated as Doxygen comments should probably have /// changed to //.

You won't need to contribute the HTML that you generate - just the changes to the JIT compiler source code itself. I hope that helps.

[1] https://www.doxygen.nl/index.html [2] https://www.doxygen.nl/manual/docblocks.html#cppblock

Aman254 commented 1 year ago

@hzongaro Thank You so, Much for this Explanation and it is also a great experience for me also as a beginner.

Now I Understand It Clearly and also I checked all the codebases after running Doxygen on it and creating a HTML Documentation that no codebase contains /// . as you mentioned

Nayem73 commented 1 year ago

@hzongaro @Aman254 hello! Is this issue closed yet? Can I be of any help?

Aman254 commented 1 year ago

No @Nayem73 its not closed yet am working on it and about to complete I'll surely be informing if required thanks for asking

TheMarvelFan commented 11 months ago

Hi is this issue still open?

Aman254 commented 11 months ago

https://github.com/eclipse-openj9/openj9/issues/17315#issuecomment-1629000617

jainyash0007 commented 7 months ago

Hello, I would like to work on this issue, if its still open and no one is working on it. @hzongaro @Aman254

hzongaro commented 7 months ago

Hello, I would like to work on this issue, if its still open and no one is working on it.

Hi, @jainyash0007. Thank you for offering to help!

I've spoken with @Aman254 off-line, and he indicated that he would still like to work on completing this work item. I had been giving him feedback on his draft pull request, but I have been busy with other things recently, so I hadn't been able to help him get that work to completion.

I would encourage you to take a look at some of the other open issues for one that looks like it might be a good candidate for a newcomer.